Skip to content

Change declarative config pojos from with<Prop> to set<Prop> - #8742

Open
jack-berg wants to merge 1 commit into
open-telemetry:mainfrom
jack-berg:declarative-config-pojo-setters
Open

Change declarative config pojos from with<Prop> to set<Prop>#8742
jack-berg wants to merge 1 commit into
open-telemetry:mainfrom
jack-berg:declarative-config-pojo-setters

Conversation

@jack-berg

@jack-berg jack-berg commented Aug 21, 2026

Copy link
Copy Markdown
Member

Related to #8402

Currently, declarative pojos have <PojoType> with<Prop>(<PropType> propName) methods for setting properties on mutable pojo types in a fluid manner. This is problematic, because with<Prop> heavily implies the the result is an immutable copy with the property changed.

This PR changes to <PojoType> set<Prop>(<PropType> propName), which I think is the best option for us:

  • Model classes stay mutable. Changing to immutable instances is much more invasive. It requires constructors which know which fields are required at initialization time, and the API changes when that changes.
  • Keep the fluent API. I.e. return the pojo type and not void. This means that these aren't valid java beans. But we don't need them to be java beans and the ergonomic benefit to keeping the fluent API is extremely good.
  • No builders. Builders would be another option, but add to an already large API surface area. Overkill for what we're doing here.

Despite the seeming large size of the PR, there is only one meaningful file changed in this PR: https://github.com/open-telemetry/opentelemetry-java/pull/8742/changes#diff-7a86335581560379612dfc344b0c7da937cec60da747e29f4873386fd9473f2f

The rest are find/replace.

@jack-berg
jack-berg requested a review from a team as a code owner August 21, 2026 21:10
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 21, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-08-21 22:09 UTC

Review the latest changes.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (b421aed) to head (1b20f6a).

Files with missing lines Patch % Lines
...iveconfig/model/internal/SamplerModelAccessor.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8742   +/-   ##
=========================================
  Coverage     91.28%   91.28%           
- Complexity    10473    10474    +1     
=========================================
  Files          1006     1006           
  Lines         28277    28277           
  Branches       3569     3569           
=========================================
+ Hits          25812    25813    +1     
  Misses         1674     1674           
+ Partials        791      790    -1     

☔ View full report in Codecov by Harness.
📢 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.

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.

1 participant