Resource in AutoConfiguredOpenTelemetrySdk object is configured from the 'resource' YAML node #7418
Conversation
…igured from the 'resource' YAML node
Improved createResource method
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7418 +/- ##
=========================================
Coverage 89.75% 89.75%
- Complexity 6980 6983 +3
=========================================
Files 797 797
Lines 21165 21180 +15
Branches 2057 2057
=========================================
+ Hits 18996 19011 +15
Misses 1505 1505
Partials 664 664 ☔ 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.
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.