Remove `GetExtensions` from `component.Host`

[bogdandrutu]

Remove GetExtensions from component.Host. This should probably be an "optional" interface in the Extension package, so that it can return GetExtensions() map[component.ID]extension.Extension.

[mx-psi]

@bogdandrutu I feel like this should stay in component.Host: this feels like the essential functionality a component.Host must have in order to support basic use cases like authentication, encoding or storage configuration.

[jpkrohling]

Isn't this what we use in confighttp and configgrpc in order to get the available auth extensions? We also want to follow the same pattern to implement middleware extensions (#7441).

[mx-psi]

@jpkrohling This would still be available, just as an optional interface.

Right now:

var host component.Host = getComponentHostSomehow()

doSomethingWithExtensions(host.GetExtensions())

After the change proposed by @bogdandrutu we would have an interface like this:

type GetExtensionsInterface interface {
  GetExtensions() map[[component.ID](component.Component)   
}

and you would use it like this:

var host component.Host = getComponentHostSomehow()

getExtHost, ok := host.(GetExtensionsInterface)

if !ok {
  panic("host does not support GetExtensions")
}

doSomethingWithExtensions(host.GetExtensions())

This would be similar to componentstatus.Reporter

[jpkrohling]

Got it -- I'm still leaning towards having it as part of the main interface, unless there's a concrete reason to not have it there. I agree with your previous statement:

this feels like the essential functionality

[jade-guiton-dd]

One possible argument for making this interface optional is that it gives us more flexibility to change the method signature later, even after stabilization, for instance if returning a map no longer seems like the right choice. (Considering we removed GetExporters from Host way back it doesn't seem implausible.) Instead of supporting the old signature forever, it could be removed without completely breaking components relying on it, though at the cost that these components will likely assume there are no extensions set up. On the other hand, if we're fine with that last part, keeping the old signature around with a dummy implementation would not be a difficult alternative either... for that reason, I don't have strong feelings one way or another.

[TylerHelmuth]

This functionality is the is only concrete function on the host interface. I also agree is it the core feature that host is providing to components. If we removed it what would we do with the host interface?

What problem is being solved by removing it?

[bogdandrutu]

What problem is being solved by removing it?

There are multiple problems:

1. We are adding the notion of "extension" within the component package which should be agnostic of different component types (ideally).
2. It is inconsistent experience because GetExtensions() returns a Component not an Extension.

[TylerHelmuth]

It is inconsistent experience because GetExtensions() returns a Component not an Extension.

If it returned an extension.Extension, would that change how the returned value is used?

Doing a cursory glance through contrib the returned value is asserted against the desired extension interface and not as an extension.Extension. This makes sense since extension.Extension is currently exactly component.Component, so there are no additional extension-specific methods to invoke.

Are you anticipating any breaking changes to extension.Extension that would differentiate it from component.Component so that getting an extension.Extension from GetExtensions() is meaningful? Unless we add methods to extension.Extension I believe users will still need to assert against their desired interface.

[mx-psi]

We are adding the notion of "extension" within the component package which should be agnostic of different component types (ideally).

@bogdandrutu component is not agnostic to component types: we define component.Kind.

[bogdandrutu]

component is not agnostic to component types: we define component.Kind.

I think now component type can be in service?

[mx-psi]

component is not agnostic to component types: we define component.Kind.

I think now component type can be in service?

Moving component.Kind to service.Kind introduces a couple of cycles that are hard to solve:

• componentstatus and service, since service/extensions keep a map of componentstatus.InstanceID and componentstatus.InstanceID depend on service.Kind. I tried moving componenstatus.InstanceID to xextension` as suggested by a comment on that package but that doesn't help.
• componenttest and service, since the componenttest defines a GetFactory method.

Maybe more? I guess this could always be moved to its own top-level package but I don't see the point right now. All in all, I am not necessarily against making the component module agnostic to component kinds but I would like to see

1. What the concrete proposal is from your side @bogdandrutu
2. What are the benefits of making component agnostic to kinds now

[mx-psi]

We discussed this on the 2025-01-27 stability meeting. Some thoughts:

• It doesn't seem like having a top-level package would make sense
• The two things Bogdan wants to look into before leaving as is are:
  • If we leave component.Kind where it is, look into whether adding new component kinds without modifying component would be feasible
  • component.Component shouldn't be equal to extension.Extension, it should be the same as in receiver.Logs (embedded).

I will take a look at both of these points, if there is a way around it we can close this as wontfix

[mx-psi]

• component.Component shouldn't be equal to extension.Extension, it should be the same as in receiver.Logs (embedded).

I believe this can be done, see #12208

[mx-psi]

If we leave component.Kind where it is, look into whether adding new component kinds without modifying component would be feasible

For this point: this is doable today, since a component.Kind is just an int and anybody can create a new kind by doing component.Kind(<number>). I filed #12214 to change it to a struct. I am not convinced this is very useful, but we may as well do it. I marked this PR as closing this issue since it would address the second and last point from https://github.com/open-telemetry/opentelemetry-collector/issues/11443#issuecomment-2616396687

[mx-psi]

I am closing this as completed with the merge of #12214