Resource in AutoConfiguredOpenTelemetrySdk object is configured from the 'resource' YAML node #7418
Conversation
…igured from the 'resource' YAML node
Improved createResource method
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7418 +/- ##
=========================================
Coverage 90.01% 90.01%
- Complexity 7080 7083 +3
=========================================
Files 803 803
Lines 21417 21432 +15
Branches 2086 2086
=========================================
+ Hits 19278 19293 +15
Misses 1477 1477
Partials 662 662 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return AutoConfiguredOpenTelemetrySdk.create( | ||
| sdk, Resource.getDefault(), null, configProvider); | ||
|
|
||
| Resource configuredResource = createResourceFromModel(model, componentLoader); |
There was a problem hiding this comment.
This means that we are calling ResourceFactory twice, but the alternative is for DeclarativeConfiguration#create to return a tuple of OpenTelemetrySdk and Resource such that the resource is accessible, which isn't great either.
I actually want to get rid of the incubating API to access the autoconfigured resource. Its come up in a variety of conversations (including this comment), but the TL;DR is:
The problem with accessing the autoconfigured resource is that in all the cases I've seen so far, its always been the wrong thing to do.
There was a problem hiding this comment.
Returning tuple from DeclarativeConfiguration#create would actually require updates of OpenTelemetryConfigurationFactory#create. This would also require some additional classes and maybe reane of OpenTelemetryConfigurationFactory.
I think instantiating resource twice is good enough solution.
Regarding the change you mentioned:
I actually want to get rid of the incubating API to access the autoconfigured resource
Do you plan to get rid of the resource property and getResource metod from AutoConfiguredOpenTelemetrySdk class?
If yes, then it may cause lots of issues in some vendor's code. We use it in our code in few places.
There was a problem hiding this comment.
If yes, then it may cause lots of issues in some vendor's code. We use it in our code in few places.
I know, but why? I'm still searching for a compelling reason vendors do this. Every instance I've seen so far has been dubious.
There was a problem hiding this comment.
Hey @robsunday you may be interested in this proposal and conversation: https://github.com/open-telemetry/opentelemetry-specification/pull/4565/files#r2193326632
There was a problem hiding this comment.
The agent is using the resource as well: https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-java-instrumentation%20ConfiguredResourceAttributesHolder&type=code
|
I've create an alternative PR that doesn't expose It also runs the customizers - which is a bug in this PR. |
|
More refined solution to the issue provided in #7639 |
3 participants
Until now the resource stored as a property of AutoConfiguredOpenTelemetrySdk object was always a resource obtained by calling Resource.getDefault(), which means it was not set up basing on content of YAML config file.
This PR is a proposal of fix for this issue.