Skip to content

Comments

docs(config): Document ObjectMapper config#8075

Merged
jack-berg merged 2 commits intoopen-telemetry:mainfrom
honeycombio:mike/move-objectmapper-to-internal
Feb 17, 2026
Merged

docs(config): Document ObjectMapper config#8075
jack-berg merged 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 11 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 😄

Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@jack-berg jack-berg merged commit 40372f4 into open-telemetry:main Feb 17, 2026
27 checks passed
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