Expose telemetry attributes via a factory option

[atoulme]

Use a factory option to set immutably the telemetry attributes on the component on creation, rather than regenerating metric/trace/log providers when those change.

This PR builds on #13827 - refactoring all the way to remove dependency on the OpenTelemetry SDK for components.

[codecov]

Codecov Report

❌ Patch coverage is 96.06299% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.42%. Comparing base (37a3ace) to head (9e9a3da).
⚠️ Report is 43 commits behind head on main.

Files with missing lines	Patch %	Lines
service/extensions/extensions.go	62.50%	2 Missing and 1 partial ⚠️
service/internal/telemetry/logger_zap.go	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13832      +/-   ##
==========================================
+ Coverage   91.38%   91.42%   +0.04%     
==========================================
  Files         647      646       -1     
  Lines       42597    42676      +79     
==========================================
+ Hits        38927    39018      +91     
+ Misses       2845     2835      -10     
+ Partials      825      823       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mx-psi]

It would be good to have an issue motivating this and #13827. It's a bit difficult to understand for me right now what the end goal is

[jade-guiton-dd]

It sounds like the goal is to remove the dependency on the "official" SDK in packages that shouldn't rely on it like component. Looking at go mod why, it looks like the dependency chain is:

go.opentelemetry.io/collector/component
# we reexport `TelemetrySettings` from `internal/telemetry`, which allows us to add an unexported field to keep track of the current set of scope attributes
go.opentelemetry.io/collector/internal/telemetry
# component attribute is a package inside the `internal/telemetry` module
go.opentelemetry.io/collector/internal/telemetry/componentattribute
# `componentattribute` imports the official trace SDK so it can provide SDK-specific methods to components like `zpagesextension` which rely on then
go.opentelemetry.io/otel/sdk/trace 
go.opentelemetry.io/otel/sdk

(Code link for the part which imports the trace SDK)

I think there are probably ways to remove this dependency chain without splitting componentattribute's functionality in two or making it harder to use (in the previous PR, by requiring two function calls to remove a scope attribute; in this one, by making it so scope attributes can only be modified at component creation).

Moreover, since service uses componentattribute independently of what SDK is being instantiated (to add the wrappers), this still wouldn't allow building a full Collector without a dependency on the SDK.

One possible solution would be to expect components to only rely on the OTel API, never SDK-specific methods, and drop the SDK-specific TracerProvider wrapper in componentattribute. This would however break the zpagesextension since it relies on SpanProcessor, which are not part of the API.

It may also be possible to allow access to SDK-specific methods through our wrappers without making the wrappers depend on the SDK code-wise. I think I tried a generic-based approach at one point and it didn't work, but maybe it's possible with some reflection/unsafe magic? I can try to look into it. Ideally this might even allow accessing SDK-specific methods from other SDKs.

[jade-guiton-dd]

After searching for a bit, it seems what I wanted to do with tracerProviderWithAttributesSdk (wrap a type to extend it, without hiding methods that aren't part of a known interface) is a recurring issue in Go, and I haven't found a way to do it without depending on the SDK, even with generics (you can't embed a generic T) or reflect (custom struct types don't promote methods from embedded fields).

I think that, unless we're willing to give up on injecting component-identifying attributes, or on being able to build the Collector without dependencies on the official SDK, our best option would be to:

• remove the tracerProviderWithAttributesSdk, which (maybe along with some changes in the tests?) would remove the SDK dependency
• provide a new special API which allows "unwrapping" the tracerProviderWithAttributes (kind of like errors.As in a sense)
• modify the rare few components that use SDK-specific methods like zpagesextension to use that new API.

I think this would be fairly reasonable, since I don't think we currently make any promises about passing the telemetry providers to components as-is. It might constitute a breaking chance however.

[atoulme]

It would be good to have an issue motivating this and #13827. It's a bit difficult to understand for me right now what the end goal is

I have filed #13842 to help. I hope it helps and doesn't add to confusion.

[atoulme]

It sounds like the goal is to remove the dependency on the "official" SDK in packages that shouldn't rely on it like component.

The impetus for me was the otelzap bridges dependency. This brought over a slew of concerns.

I think there are probably ways to remove this dependency chain without splitting componentattribute's functionality in two or making it harder to use (in the previous PR, by requiring two function calls to remove a scope attribute; in this one, by making it so scope attributes can only be modified at component creation).

Was the goal to allow changing scope attributes after component creation? That doesn't seem like a requirement we'd want to go after to me, is there a use case I am missing?

Moreover, since service uses componentattribute independently of what SDK is being instantiated (to add the wrappers), this still wouldn't allow building a full Collector without a dependency on the SDK.

If you're going to build a full collector, import the service module and deal with the dependencies. That's not the case for most components however, and it's possible to build your own binary without the service module.

One possible solution would be to expect components to only rely on the OTel API, never SDK-specific methods, and drop the SDK-specific TracerProvider wrapper in componentattribute. This would however break the zpagesextension since it relies on SpanProcessor, which are not part of the API.

Is this PR not a possible solution as well?

It may also be possible to allow access to SDK-specific methods through our wrappers without making the wrappers depend on the SDK code-wise. I think I tried a generic-based approach at one point and it didn't work, but maybe it's possible with some reflection/unsafe magic? I can try to look into it. Ideally this might even allow accessing SDK-specific methods from other SDKs.

I don't follow, sorry.

[jade-guiton-dd]

I posted a response in the issue thread to keep the discussion organized. Hopefully it clarifes my concerns about the approach in these two PRs, and better explains why the trace provider wrapper currently depends on the trace SDK.

[github-actions]

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