Vendor specific instrumentation config options handling #14016

robsunday · 2025-06-10T18:28:02Z

Some vendors create their own Java Agent distribution with their proprietary extensions. These extensions usually require some custom configuration properties.
These PR is a proposal of supporting custom configuration properties in file based declarative configuration.

Current YAML structure and the implementation of DeclarativeConfigPropertiesBridge supports only config properties with name prefix 'otel.instrumentation'. These properties must be put under:

instrumentation/development:
  java:

When such config property (like otel.instrumentation.kafka.producer-propagation.enabled) is put into the following YAML structure:

instrumentation/development:
  java:
    kafka:
      producer-propagation:
        enabled: true

We have issue if we need to put somewhere a property like: acme.server.name.
There is no obvious place now where we can put it, and in the code still retrieve it using original acme.server.name key.

The idea is to put somewhere dedicated vendor: node which would be root of unchanged property names. In this approach vendor is put under java (and possibly other languages):

instrumentation/development:
  java:
    kafka:
      producer-propagation:
        enabled: true
    vendor:
      acme:
        server:
          name: "prime"

Maybe it be better to put one 'vendor' node under 'general' instead?
In such a case config would have the following structure:

instrumentation/development:
  general:
    vendor:
      acme:
        server:
          name: "prime" 
  java:
    kafka:
      producer-propagation:
        enabled: true

robsunday · 2025-06-10T18:29:06Z

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java

@@ -30,25 +26,4 @@ public interface AgentListener extends Ordered {
   */
  void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk);

-  /** Resolve {@link ConfigProperties} from the {@code autoConfiguredOpenTelemetrySdk}. */
-  static ConfigProperties resolveConfigProperties(


[for reviewer] This was extracted to separate public class (see ConfigPropertiesUtil) so can be used also for BeforeAgentListener's

robsunday · 2025-06-10T18:30:19Z

...pi/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java

@@ -52,9 +52,12 @@ final class DeclarativeConfigPropertiesBridge implements ConfigProperties {

  // The node at .instrumentation.java
  private final DeclarativeConfigProperties instrumentationJavaNode;
+  // The node at .instrumentation.java.vendor


[for reviewer] As I mentioned, we may consider moving it under 'general', so it would be .instrumentation.general.vendor'

jack-berg · 2025-06-12T15:14:25Z

...pi/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java


  DeclarativeConfigPropertiesBridge(DeclarativeConfigProperties instrumentationNode) {
    instrumentationJavaNode = instrumentationNode.getStructured("java", empty());
+    vendorNode =
+        instrumentationJavaNode.getStructured(
+            "vendor", empty()); // vendor could go to instrumentation.general instead


I don't love the idea of having a dedicated property called vendor which includes data from various vendors' config schemas. It would be better if the config could look like:

instrumentation/development: java: acme: server: name: "prime"

And then accessing ConfigProperties.getString("acme.server.name") would resolve "prime".

To accomplish this, we could introduce logic which:

If a property starts with otel.instrumentation., strip this prefix. I.e. otel.instrumentation.kafka.producer-propagation.enabled becomes kafka.producer-propagation.enabled.

Else, keep the property as is.

Resolve all properties starting at the .instrumentation/development.java node. For example:

otel.instrumentation.kafka.producer-propagation.enabled resolves to .instrumentation/development.java.kafka.producer-propagation.enabled

acme.server.name resolves to .instrumentation/development.java.acme.server.name

I see your point.
I considered this approach as well, but decided to skip it due to the risk of name collision and kind of nonuniform approach within the same node. That's why I think having a dedicated node (vendor in our case) is not so bad idea, but definitely it is not the perfect solution.

Let's think about the other approach - do not make any prefix manipulation for otel.instrumentation.* and nest them in 1:1 yaml structure:

instrumentation/development: java: otel: instrumentation: kafka: producer-propagation: enabled: true micrometer: histogram-gauges: enabled: true acme: server: name: "prime"

This looks a bit awkward doe to repeating instrumentation, but it is repeated only once for all the properties. If this is an issue then we may consider renaming instrumentation/development: to something else, like instrumentation_config, config_properties or just config + /development for now.
This would support unified approach for standard and custom properties in yaml structure and in the code.
I know this is a big change, but still possible and worth reconsidering.
What do you think about it?

jack-berg

I think this is a good thing to improve.

But I'll note that if motivated, a distribution has the tools to solve this today.

Consider a distribution which has a vendor specific property to configure the server name.

In the flat config scheme, users configure it like:

acme.server.name=prime

In the declarative config scheme, users configure it like:


instrumentation/development:
  java:
    acme:
      server:
        name: prime

For a distribution to handle both scenarios, they can build an abstraction which works whether AutoConfiguredOpenTelemetrySdk returns ConfigProperties or DeclarativeConfigProperties.

A naive implementation:

// non-null when flat config is used
@Nullable private final ConfigProperties configProperties =  AutoConfigureUtil.getConfig(autoConfiguredSdk);

// non-empty when declarative config is used
private final DeclarativeConfigProperties declarativeConfigProperties =
        Optional.ofNullable(AutoConfigureUtil.getConfigProvider(autoConfiguredSdk))
            .map(ConfigProvider::getInstrumentationConfig)
            .orElse(empty());

String getServerName() {
  return configProperties != null
    ? configProperties.getString("acme.server.name")
    : declarativeConfigProperties
        .getStructured("acme", empty())
        .getStructured("server", empty())
        .getString("name");

}

The benefit of introducing code like this PR does is that it makes it simpler for distributions.

The downside of distributions relying on this is that they loose the benefit of being able to express complex config schemas that comes with declarative config, limited to maps of maps and unable to express things like arrays of maps like we see in things like the rule based routing sampler.

robsunday · 2025-06-13T11:21:45Z

Thank you for this explanation. So let's agree on the approach on how vendor specific config properties are stored in YAML file and I'll then update the PR.

robsunday · 2025-06-17T15:07:34Z

...rc/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java

+    // as for otel.instrumentation.*)
+    assertThat(bridge.getBoolean("acme.full_name.preserved")).isTrue();
+    // Example of property name collision:
+    assertThat(bridge.getBoolean("acme.full_name.preserved"))


[for reviewer] This is the downside of the approach without .vendor
It is for illustration purposes only and will be removed

Seems alright to me. What do @open-telemetry/java-instrumentation-approvers think?

seems like a good compromise to me 👍

jack-berg · 2025-06-17T17:30:49Z

...t-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java

+
+      if (instrumentationConfig != null) {
+        return new DeclarativeConfigPropertiesBridge(instrumentationConfig);
+      }


Noticing a bug in here that preceded the re-organization in this PR. getInstrumentationConfig() may be null when declarative config is used but the instrumentation/development: node is not set. This is perfectly valid, but I believe results in the exception on line 37 being thrown. Solution is:

if (instrumentationConfig == null) { instrumenationConfig = DeclarativeConfigProperties.empty(); } return new DeclarativeConfigPropertiesBridge(instrumentationConfig);

I'll apply this fix

jack-berg · 2025-06-17T17:33:42Z

...pi/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java

@@ -130,6 +130,30 @@ public Map<String, String> getMap(String propertyName) {
  @Nullable
  private <T> T getPropertyValue(
      String property, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
+    T value = getOtelPropertyValue(property, extractor);
+    if (value == null) {


Could achieve this same effect without duplicating the logic to split on . and walk the segments by checking if the property starts with OTEL_INSTRUMENTATION_PREFIX, and if so, stripping it. If not, proceed.

Vendor specific instrumentation config options handling

16da1b0

robsunday commented Jun 10, 2025

View reviewed changes

spotless

c707f0e

This comment was marked as resolved.

Sign in to view

jack-berg reviewed Jun 12, 2025

View reviewed changes

robsunday added 2 commits June 17, 2025 16:58

Custom properties are no longer under "vendor" node.

73b1797

UT update

506425b

robsunday commented Jun 17, 2025

View reviewed changes

jack-berg reviewed Jun 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Vendor specific instrumentation config options handling #14016

Vendor specific instrumentation config options handling #14016

Uh oh!

robsunday commented Jun 10, 2025 •

edited

Loading

Uh oh!

robsunday Jun 10, 2025 •

edited

Loading

Uh oh!

robsunday Jun 10, 2025 •

edited

Loading

Uh oh!

This comment was marked as resolved.

jack-berg Jun 12, 2025

Uh oh!

robsunday Jun 13, 2025

Uh oh!

jack-berg left a comment

Uh oh!

robsunday commented Jun 13, 2025

Uh oh!

robsunday Jun 17, 2025

Uh oh!

jack-berg Jun 17, 2025

Uh oh!

trask Jun 17, 2025

Uh oh!

jack-berg Jun 17, 2025

Uh oh!

robsunday Jun 17, 2025

Uh oh!

jack-berg Jun 17, 2025

Uh oh!

Uh oh!

Vendor specific instrumentation config options handling #14016

Are you sure you want to change the base?

Vendor specific instrumentation config options handling #14016

Uh oh!

Conversation

robsunday commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robsunday Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robsunday Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jack-berg left a comment

Choose a reason for hiding this comment

Uh oh!

robsunday commented Jun 13, 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!

Uh oh!

robsunday commented Jun 10, 2025 •

edited

Loading

robsunday Jun 10, 2025 •

edited

Loading

robsunday Jun 10, 2025 •

edited

Loading