Align SemconvStability with newer declarative config#18908
Conversation
20a9cf4 to
309f180
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates SemconvStability to recognize the newer declarative config model for semantic convention stability selection, including parsing general.stability_opt_in_list, supporting per-domain *.semconv settings, and adding unit tests to validate selection behavior and precedence.
Changes:
- Add parsing for declarative
stability_opt_in_listand refactor selection into aSemconvSelectionhelper. - Add per-domain declarative semconv selection support (
db,code,rpc,messaging) via{domain}.semconv.versionand{domain}.semconv.dual_emit. - Add focused unit tests for parsing and precedence/selection behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java | Refactors semconv stability resolution to incorporate newer declarative config and per-domain selection. |
| instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/SemconvStabilityTest.java | Adds unit tests covering parsing and selection precedence across declarative/legacy/flat config paths. |
eeebbc5 to
ad419e0
Compare
Layered on top of open-telemetry#18936: - New schema: <domain>.semconv.version (0/1) + dual_emit - New top-level stability_opt_in_list config - Precedence: domain schema > stability list > existing legacy fallback Signed-off-by: Gregor Zeitlinger <[email protected]>
ad419e0 to
5e90e21
Compare
…etry#18933 service.peer/rpc/messaging are not yet stable, so their declarative config belongs under java.common.semconv_stability.preview rather than general. System property otel.semconv-stability.preview is unchanged. Signed-off-by: Gregor Zeitlinger <[email protected]>
|
I can't approve (as I created the first version) - but I really like the new clean data model! |
| // stability_opt_in_list: "database" | ||
| // db: | ||
| // semconv: | ||
| // version: 1 |
There was a problem hiding this comment.
I doubt that it is obvious for users what version 1 and 0 mean here. Probably it is easier to understand how stability_opt_in_list works. Perhaps this isn't important anyway since these flags are unlikely to be widely used.
There was a problem hiding this comment.
this is part of declarative configuration: https://github.com/open-telemetry/opentelemetry-configuration/blob/main/snippets/ExperimentalGeneralInstrumentation_semconv_stability_opt_in.yaml
I agree for now stability_opt_in_list is simpler. this is more of future proofing for when there are subsequent major version bumps of semantic conventions
Teach
SemconvStabilityto read the newer declarativegeneral.stability_opt_in_listsetting, support per-domain declarative semconv config fordb,code,rpc, andmessaging, and movepreviewconfig for service.peer, rpc, and messaging tojava.commoninstead ofgeneral(closes #18933). This also adds focused unit tests for selection precedence and parsing logic.Config precedence is: per-domain
general.<domain>.semconv.*>general.stability_opt_in_list> fallback. Thejava.common.v3_preview=true/otel.semconv-stability.v3-preview=truesetting affects only fallback/default selection for non-messaging domains; it does not override explicit per-domainsemconv.version/dual_emitconfig.Fixes #18907, #18933.