Repository navigation
Make declarative config parsing and mapping pluggable - #8983
ash-thakur-rh wants to merge 3 commits into
Conversation
Add parser and mapper SPIs, and move the Jackson 2 mapper into an optional adapter module. Signed-off-by: Ashish Thakur <ashishth@redhat.com>
Signed-off-by: Ashish Thakur <ashishth@redhat.com>
Pull request dashboard statusWaiting on the author · refreshed 2026-11-03 23:06 UTC Respond to 6 review items (e.g. link a commit, explain why not, ask a follow-up): Status above doesn't look right?
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8983 +/- ##
============================================
+ Coverage 90.31% 90.34% +0.02%
- Complexity 10981 10997 +16
============================================
Files 1019 1023 +4
Lines 30072 30098 +26
Branches 3927 3928 +1
============================================
+ Hits 27160 27192 +32
+ Misses 1952 1944 -8
- Partials 960 962 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Ashish Thakur <ashishth@redhat.com>
CLA will be soon added to my account. |
jack-berg
left a comment
jack-berg
left a comment
There was a problem hiding this comment.
This is pretty close to the shape I imagined. Couple of things to think through tho.
There was a problem hiding this comment.
Hey let's put these in sdk-extensions/autoconfigure-spi in the io.opentelemetry.sdk.autoconfigure.spi.internal package. That's where ComponentProvider lives, which is another key SPI for declarative config. In #8981 we're promoting ComponentProvider to be stable and it will live in io.opentelemetry.sdk.autoconfigure.spi.declarativeconfig as its permanent home.
There was a problem hiding this comment.
Better to include the jackson major version number in the module name. I.e. declarative-config-jackson2. We learned the hard was with okhttp sender that people have a real problem with major version updates with these kind of load bearing dependencies. Embedding the major version gives us flexibility to maintain two versions at the same time to keep everyone happy. Can of course start without the version in the name and rename to declarative-config-jackson3 if/when needed, but its a nice visual queue to start with.
Also put the major version in the package name, i.e. io.opentelemetry.sdk.autoconfigure.declarativeconfig.jackson2.
| @@ -0,0 +1,16 @@ | |||
| plugins { | |||
| id("otel.java-conventions") | |||
| id("otel.publish-conventions") | |||
There was a problem hiding this comment.
| id("otel.publish-conventions") | |
| id("otel.publish-conventions") | |
| id("otel.animalsniffer-conventions") |
| * Converts parsed configuration values to declarative configuration models and generic maps. | ||
| * | ||
| * <p>Implementations are discovered with {@link java.util.ServiceLoader}. The implementation with | ||
| * the highest {@link #order()} is used. The Jackson 2 implementation is supplied by the {@code |
There was a problem hiding this comment.
The typical pattern we use is:
- If exactly one is on the class path, use it
- If two are on the class path, and the user hasn't selected one via env var / system property, log a warning and use the first we find
- If two are on the class path, and the user has selected one via env var / system property, use it
| * This class is internal and is hence not for public use. Its APIs are unstable and can change at | ||
| * any time. | ||
| */ | ||
| public final class SnakeYamlDeclarativeConfigurationParser |
There was a problem hiding this comment.
Interesting.. so we have a dedicated jackson module, but build the snakeyaml implementation into declarative-config directly despite it being pluggable via SPI.
I think we need to think this through and be consistent.
| * Declarative config: Add parser and mapper SPIs in a dedicated artifact for issue | ||
| [#8820](https://github.com/open-telemetry/opentelemetry-java/issues/8820). Include the Jackson 2 | ||
| mapper by default through a runtime adapter that can be excluded to register a custom mapper such | ||
| as one based on Jackson 3. |
There was a problem hiding this comment.
No need for changelog entries. These are generated as part of release notes.
Description:
Closes #8820