add method to retrieve instrumentation configuration by name #7927

zeitlinger · 2025-12-17T07:00:23Z

Follow up to #7923

See open-telemetry/opentelemetry-java-instrumentation#15641 (comment)

codecov · 2025-12-17T07:06:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.09%. Comparing base (22d90b6) to head (576f287).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7927      +/-   ##
============================================
- Coverage     90.10%   90.09%   -0.02%     
- Complexity     7370     7438      +68     
============================================
  Files           828      834       +6     
  Lines         22254    22541     +287     
  Branches       2192     2238      +46     
============================================
+ Hits          20053    20309     +256     
- Misses         1515     1532      +17     
- Partials        686      700      +14

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

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java

jack-berg · 2025-12-18T20:11:41Z

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java

+   * @param name the name of the instrumentation
+   * @return the {@link DeclarativeConfigProperties} for the given instrumentation name
+   */
+  default DeclarativeConfigProperties getJavaInstrumentationConfig(String name) {


In the java SIG today we discussed adding an additional helper DeclarativeConfigProperties getGeneralInstrumentationConfig() which would operate the same but access the .instrumentation/development.general node.

Your call whether we do that in a separate PR or this one.

Would be nice to also have a getDistributionConfig() to access the .distribution node (without null worries)

That's a different story. Right now the scope of what ConfigProvider knows about is limited to the .instrumentation/development node. Would need to have something like open-telemetry/opentelemetry-specification#4800 to expand the scope.

Note its still possible for distributions to access .distribution without PR#4800 with a pattern like:

OpenTelemetryConfigurationModel model = parse(new File(System.getEnv("OTEL_EXPERIMENTAL_CONFIG_FILE"))); DistributionModel distribution = model.getDistribution(); ExtendedOpenTelemetrySdk sdk = create(model);

that pattern is brittle, e.g. doesn't work with the spring starter

@zeitlinger in spring starter can't we get the distribution node from https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/dcee1b769f353bee881ab7be8c7caa7715ea5143/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfiguration.java#L158?

Good point, that would work for spring applications.

AgentListener passes AutoConfiguredOpenTelemetrySdk - and that gives you access to ConfigProvider only - but Jacks suggestion would work

if the user also checks for the corresponding system property

...src/testIncubating/java/io/opentelemetry/sdk/autoconfigure/DeclarativeConfigurationTest.java

… name

zeitlinger · 2025-12-19T10:36:52Z

@jack-berg added the second method for "general" and improved test coverage

jack-berg · 2025-12-19T14:39:42Z

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java

+   * @return the {@link DeclarativeConfigProperties} for the given general instrumentation config
+   *     name
+   */
+  default DeclarativeConfigProperties getGeneralInstrumentationConfig(String name) {


This works because for now there are no top level properties under .instrumentation/development.general. But if there were, callers would be unable to access them. I think we should omit the String name parameter for now and just return the .instrumentation/development.general node.

oh good point, can .instrumentation/development.java. have top-level properties?

I really don't mind getJavaInstrumentationConfig().get(name)

This is only a shortcut method - so I think we're allowed to nudge towards what we think is best practice.
I think that is the currently implemented solution, but I can live with either.

oh good point, can .instrumentation/development.java. have top-level properties?

No - its properties are limited to objects: https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema-docs.md#experimentallanguagespecificinstrumentation-

In contrast, .general has explicit properties of http and peer which are objects, but nothing stating all properties are objects: https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema-docs.md#experimentalgeneralinstrumentation-

ok, I'll adapt "general" then

trask · 2025-12-19T19:37:25Z

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java

+   *
+   * @return the {@link DeclarativeConfigProperties} for the general instrumentation configuration
+   */
+  default DeclarativeConfigProperties getGeneralInstrumentationConfig() {


would we be comfortable with something shorter, e.g.

getInstrumentationGeneral()

getInstrumentationJava(name)

What about "getGeneralInstrumentation"?

What about "getGeneralInstrumentation"?

I like this for general, but symmetric naming yields getJavaInstrumentation(name), which makes it sound like its accessing the instrumentation rather than the config for the instrumentation.

Not that getInstrumentationJava(name) is especially obvious either, but at least the method name reflects the path to the YAML node being accessed.

maybe

getInstrumentation(name)

getGeneralInstrumentation()

with the idea that the "Java" is redundant for the Java SDK

Is getInstrumentation(name) too close to getInstrumentationConfig() given the difference in function?

is there a way we could remove getInstrumentationConfig()?

what if we added something like .exists() to DeclarativeConfigurationProperties for the few people who may care about the difference between intermediate nodes being present or not

I'm not particularly attached to it, but there's the bit about it being part of the spec. Of course the spec can be changed. But the path of least resistance for the short term is to find method names that jive with the current spec and which can be characterized as syntactic sugar. The spec doesn't prohibit syntactic sugar so it's the sweet spot where we can improve usability quickly.

I went for

getInstrumentationConfig

getGeneralInstrumentationConfig

now based on the feedback. Let me know what you think.

zeitlinger self-assigned this Dec 17, 2025

zeitlinger requested a review from a team as a code owner December 17, 2025 07:00

zeitlinger added this to Declarative Configuration: Java Dec 17, 2025

zeitlinger mentioned this pull request Dec 17, 2025

The one true configuration bridge open-telemetry/opentelemetry-java-instrumentation#15641

Merged

jack-berg reviewed Dec 17, 2025

View reviewed changes

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java Outdated Show resolved Hide resolved

zeitlinger mentioned this pull request Dec 18, 2025

use DeclarativeConfigUtil for library config usage open-telemetry/opentelemetry-java-instrumentation#15656

Open

jack-berg reviewed Dec 18, 2025

View reviewed changes

...src/testIncubating/java/io/opentelemetry/sdk/autoconfigure/DeclarativeConfigurationTest.java Show resolved Hide resolved

zeitlinger added 5 commits December 19, 2025 11:29

feat: add method to retrieve instrumentation configuration by name

3d18a77

add test

704499d

pr review

85beb5c

feat: add method to retrieve general instrumentation configuration by…

5b60d37

… name

feat: add method to retrieve general instrumentation configuration by…

051899e

… name

zeitlinger force-pushed the get-java-node branch from d4d2658 to 051899e Compare December 19, 2025 10:33

jack-berg reviewed Dec 19, 2025

View reviewed changes

pr review

2283eb6

trask reviewed Dec 19, 2025

View reviewed changes

pr review

576f287

trask mentioned this pull request Dec 21, 2025

Update DeclarativeConfigUtil to support general node open-telemetry/opentelemetry-java-instrumentation#15704

Merged

add method to retrieve instrumentation configuration by name #7927

Are you sure you want to change the base?

add method to retrieve instrumentation configuration by name #7927

Uh oh!

Conversation

zeitlinger commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zeitlinger commented Dec 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zeitlinger commented Dec 17, 2025 •

edited

Loading

codecov bot commented Dec 17, 2025 •

edited

Loading