Deprecate component.Host.GetExporters

[djaglowski]

See #7370

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (87dd85a) 91.02% compared to head (4ffb16f) 91.02%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7390   +/-   ##
=======================================
  Coverage   91.02%   91.02%           
=======================================
  Files         295      295           
  Lines       14535    14535           
=======================================
  Hits        13231    13231           
  Misses       1042     1042           
  Partials      262      262           

Impacted Files	Coverage Δ
service/host.go	89.47% <ø> (ø)
service/internal/graph/graph.go	98.68% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[mx-psi]

LGTM, should we also deprecate

opentelemetry-collector/service/host.go

Line 70 in 042b876

func (host *serviceHost) GetExporters() map[component.DataType]map[component.ID]component.Component {

and

opentelemetry-collector/service/graph.go

Line 318 in 042b876

func (g *pipelinesGraph) GetExporters() map[component.DataType]map[component.ID]component.Component {

to avoid further internal usage of these methods?

Also, we may need to add exceptions for existing components on contrib that use this method while we migrate them

[bogdandrutu]

There are some usages that cannot be converted I believe, one is in the signalfx receiver. cc @dmitryax

[dmitryax]

There are some usages that cannot be converted I believe, one is in the signalfx receiver. cc @dmitryax

At Splunk, we rely on metadata updates sent directly from receivers to exporters. The same approach is implemented in k8scluster receiver, which we have an issue for open-telemetry/opentelemetry-collector-contrib#19741. The effort to replace that mechanism is significant. So maybe we wait for an alternative implementation before deprecating host.GetExporters?

[dmitryax]

Also, we usually resolve all the to-be-deprecated usages in contrib before the actual deprecation in the core. Then, the deprecated API is usually removed on the next release. That way, we avoid adding //nolint comments in contrib. This deprecation should definitely stay longer than one version, but I still think deprecating it now is too aggressive. I'd like to have at least some solution for the metadata updates first.

[djaglowski]

There are some usages that cannot be converted I believe, one is in the signalfx receiver. cc @dmitryax

#7370 details all uses in core and contrib. k8sclusterreciever -> signalfxexporter is the only case that cannot be solved with a connector, but it can be solved with an extension.

we usually resolve all the to-be-deprecated usages in contrib before the actual deprecation in the core.

In this case at least, I wonder if by waiting to deprecate we could be inviting additional novel uses that would have to be untangled. If you think it'll be a while before k8sclusterreciever -> signalfxexporter can use an extension, perhaps we could deprecate when that is the only remaining case? Then we only have one //nolint but in exchange we could discourage misuse.

[dmitryax]

#7370 details all uses in core and contrib. k8sclusterreciever -> signalfxexporter is the only case that cannot be solved with a connector, but it can be solved with an extension.

We also use that in some receivers in Splunk distribution, but it's the same metadata use case.

In this case at least, I wonder if by waiting to deprecate we could be inviting additional novel uses that would have to be untangled. If you think it'll be a while before k8sclusterreciever -> signalfxexporter can use an extension, perhaps we could deprecate when that is the only remaining case? Then we only have one //nolint but in exchange we could discourage misuse.

Sounds good to me. Let's migrate at least a couple of components before deprecating. And I'll start planning the migration for open-telemetry/opentelemetry-collector-contrib#19741. I'm good with deprecating it earlier, but we need a more descriptive message telling users it'll stay until all the usages in contrib are migrated with a link to #7370. Currently, it seems like it'll be dropped next release as it's usually done for other deprecations.

[djaglowski]

I'm good with deprecating it earlier, but we need a more descriptive message telling users it'll stay until all the usages in contrib are migrated with a link to #7370. Currently, it seems like it'll be dropped next release as it's usually done for other deprecations.

Good points. I'll update the comment and will continue pushing forward with the other uses.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[dmitryax]

I think we can merge it. Let's just remove the target version for now?

[dmitryax]

I think we can merge it. Let's just remove the target version for now?