Extract declarative config to new opentelemetry-sdk-extension-decelar…#8265
Extract declarative config to new opentelemetry-sdk-extension-decelar…#8265jack-berg wants to merge 3 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8265 +/- ##
=========================================
Coverage 90.25% 90.25%
Complexity 7683 7683
=========================================
Files 850 850
Lines 23190 23190
Branches 2353 2353
=========================================
Hits 20930 20930
Misses 1533 1533
Partials 727 727 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ative-config module
eb396e1 to
b60c2eb
Compare
robsunday
left a comment
There was a problem hiding this comment.
Looks good with some minor comments
zeitlinger
left a comment
There was a problem hiding this comment.
Root package is io.opentelemetry.sdk.declarativeconfig - not io.opentelemetry.sdk.declarative.config as pr body says.
I do want to try out the opentelemetry-sdk-extension-autoconfigure alternative in a different PR. Will link here when done.
Here it is: #8270 That new PR has the advantage that it keeps the yaml deps optional. |
…y-java into new-declarative-config-module
Resolves #7292
Extract declarative config from
opentelemetry-sdk-extension-incubatortoopentelemetry-sdk-extension-declarative-config.Root package at
io.opentelemetry.sdk.declarativeconfig.Other options considered:
opentelemetry-sdk-config- I.e. make declarative config a peer ofopentelemetry-sdk-{signal}. The existence ofSdkConfigProvider- which currently resides inopentelemetry-sdk-allsuggests this should be the case. However, declarative config requires dependencies on each of the otheropentelemetry-sdk-{signal}modules, and has a YAML dependency which we'll want to keep opt in.opentelemetry-sdk-extension-autoconfigure- I.e. extend existing autoconfigure with declarative config. I don't hate this idea, but it would mean adding a YAML dependency to autoconfigure. As it stands currently and with this PR, autoconfigure has a compileOnly dependency on declarative config where the behavior changes at runtime if available, i.e. we start looking forOTEL_CONFIG_FILEbeing set and if so delegating config to declarative config.Coming in a followup PRs:
org.snakeyaml:snakeyaml-enginefor parse input YAML string toObject, then we use jackson to convertObjecttoOpenTelemetryConfigurationModel. Iforg.snakeyaml.snakeyaml-enginehad unsolved vulnerabilities or if breaking changes caused runtime errors from the diamond dependency problem, then users would have no recourse. We could make these functions pluggable via SPI and try to provide at least one other implementation.