Skip to content

docs(config): Document ObjectMapper config#8075

Open
MikeGoldsmith wants to merge 2 commits intoopen-telemetry:mainfrom
honeycombio:mike/move-objectmapper-to-internal
Open

docs(config): Document ObjectMapper config#8075
MikeGoldsmith wants to merge 2 commits intoopen-telemetry:mainfrom
honeycombio:mike/move-objectmapper-to-internal

Conversation

@MikeGoldsmith
Copy link
Member

@MikeGoldsmith MikeGoldsmith commented Feb 12, 2026

Summary

Documents ObjectMapper configuration for external consumers via copy-paste approach instead of exposing internal API. Per maintainer feedback, avoids coupling by providing documentation rather than API access.

Addresses #7843

Background

Spring starter needs same ObjectMapper config to parse OpenTelemetry YAML. Three approaches considered:

  1. Copy-paste (current in instrumentation)
  2. Reflection
  3. Internal API (original PR approach)

Changes

  • Added "For Implementers" section to DeclarativeConfiguration javadoc with complete ObjectMapper setup example
  • Added field-level javadoc on MAPPER explaining configuration
  • Documented why each setting exists (empty objects for nulls, boxed primitives stay null)

Testing

  • No API changes, only documentation additions

Spring starter needs same ObjectMapper config but can't access
package-private field without reflection. New public internal class
provides access.

Fixes open-telemetry#7843
@MikeGoldsmith MikeGoldsmith requested a review from a team as a code owner February 12, 2026 13:49
@MikeGoldsmith MikeGoldsmith changed the title fix(config: Move ObjectMapper to internal YamlObjectMapper class. fix(config): Move ObjectMapper to internal YamlObjectMapper class Feb 12, 2026
@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.21%. Comparing base (f42ac7d) to head (c872957).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8075      +/-   ##
============================================
+ Coverage     90.20%   90.21%   +0.01%     
- Complexity     7593     7606      +13     
============================================
  Files           841      841              
  Lines         22913    22923      +10     
  Branches       2289     2291       +2     
============================================
+ Hits          20668    20680      +12     
+ Misses         1529     1526       -3     
- Partials        716      717       +1     

☔ 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.

@jack-berg
Copy link
Member

@zeitlinger / @trask is this still needed from the instrumentation side? The last comment in the issue indicates a preference for copy / paste rather than letting consumers leverage the same object mapper:

I personally prefer a bit of copy-paste over relying on internal APIs

My perspective: in addition to not relying on internal APIs, I don't like being in the business of having an API (public or internal) to access a jackson ObjectMapper. That's not our business.

Per maintainer feedback, avoid exposing ObjectMapper API (internal or public).
Instead, document configuration for copy-paste approach.

- Move ObjectMapper back from YamlObjectMapper to DeclarativeConfiguration
- Add "For Implementers" section in javadoc with setup example
- Add field javadoc explaining config and referencing class docs
- Remove YamlObjectMapper.java
@MikeGoldsmith MikeGoldsmith changed the title fix(config): Move ObjectMapper to internal YamlObjectMapper class docs(config): Document ObjectMapper config Feb 16, 2026
@MikeGoldsmith
Copy link
Member Author

MikeGoldsmith commented Feb 16, 2026

@jack-berg thanks for feedback. I misunderstood the original ask, so sticking with simple docs approach is good.

I've updated the PR to just document how to configure the ObjectMapper. Let me know what you think 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants