Using component for no op host instead of component test

[chahatsagarmain]

Which problem is this PR solving?

• ISSUE Invalid use of noopHost in prod code #6049

Description of the changes

• Created a no op host using component and removed code for component test

How was this change tested?

• Using make lint test
• Please review and suggest changes and improvements

[mahadzaryab1]

we also have a usage of componenttest.NewNopHost in https://github.com/jaegertracing/jaeger/blob/main/cmd/jaeger/internal/integration/span_writer.go#L55. Maybe we can move the NopHost to a shared package rather than just having it live here so we don't have to define it every time we need it?

[yurishkuro]

the point was to use the real Host provided by OTEL collector. There's no reason to define our own noop host.

[mahadzaryab1]

looks like we have another noop host in jaeger as well - https://github.com/jaegertracing/jaeger/blob/main/cmd/collector/app/handler/otlp_receiver.go#L185

[chahatsagarmain]

So we need host implementation here with its related methods ?

[mahadzaryab1]

@chahatsagarmain instead of creating a nophost here, you can just pass component.Host into NewFactoryWithConfig from https://github.com/jaegertracing/jaeger/blob/main/cmd/jaeger/internal/extension/jaegerstorage/extension.go#L115

[chahatsagarmain]

@mahadzaryab1 can you check the host interface commit , I have created the host interface with extensions method . do i need to add the same for factories here ?

[mahadzaryab1]

@chahatsagarmain so we actually don't need to define a host at all. if you take a look at this method, you'll see that the host is available to us already. What you need to do is pass the host down from the method I linked above into this function and you can then pass it into Initialize and then into ToClientConn. Does that make sense?

[chahatsagarmain]

Passing as a params to Initialize gives declaration error for Factory but Creating a host in NewFactoryWithConfig wouldnt that also be a no op host?

[chahatsagarmain]

@mahadzaryab1 can the otel collector initialize a component.Host in collectorParams which can be used here ?

[mahadzaryab1]

I think you'll want to do something like this:

1. Update Factory struct to contain the host

type Factory struct {
	metricsFactory metrics.Factory
	logger         *zap.Logger
	tracerProvider trace.TracerProvider
	config         Config
	services       *ClientPluginServices
	remoteConn     *grpc.ClientConn
	host           component.Host
}

2. Update NewFactoryWithConfig so that it takes in the host and sets it in the factory

func NewFactoryWithConfig(
	cfg Config,
	metricsFactory metrics.Factory,
	logger *zap.Logger,
	host component.Host,
) (*Factory, error) {
	f := NewFactory()
	f.config = cfg
	f.host = host
	if err := f.Initialize(metricsFactory, logger); err != nil {
		return nil, err
	}
	return f, nil
}

3. Use f.host in Initialize

[yurishkuro]

+1 - don't change the interface, but ok to change constructor

[mahadzaryab1]

you don't need to create a new host, its passed into this method. Just change the _ in the function signature to be host and pass that in here.