declarative config: early init and property mapping #14184

zeitlinger · 2025-07-04T16:56:16Z

Integration test already existed before:

opentelemetry-java-instrumentation/instrumentation/methods/javaagent/build.gradle.kts

Lines 22 to 36 in 4251217

    
           testing { 
        
             suites { 
        
               val declarativeConfigTest by registering(JvmTestSuite::class) { 
        
                 targets { 
        
                   all { 
        
                     testTask.configure { 
        
                       jvmArgs( 
        
                         "-Dotel.experimental.config.file=$projectDir/src/declarativeConfigTest/resources/declarative-config.yaml" 
        
                       ) 
        
                     } 
        
                   } 
        
                 } 
        
               } 
        
             } 
        
           }

...in/java/io/opentelemetry/javaagent/tooling/config/DeclarativeConfigEarlyInitAgentConfig.java

zeitlinger · 2025-07-07T08:15:05Z

@laurit @trask please have a look 😄

I will create a bunch of follow up tasks to not make this PR bigger.

zeitlinger · 2025-07-07T10:14:05Z

...in/java/io/opentelemetry/javaagent/tooling/config/DeclarativeConfigEarlyInitAgentConfig.java

+            this.configurationModel, SpiHelper.serviceComponentLoader(extensionClassLoader));
+    Runtime.getRuntime().addShutdownHook(new Thread(sdk::close));
+    GlobalOpenTelemetry.set(sdk);
+    GlobalConfigProvider.set(this.configProvider);


created #14191 for the corresponding instrumentation

zeitlinger · 2025-07-07T11:01:05Z

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

+        "otel.javaagent.logging.application.logs-buffer-max-records",
+        "agent.logging.output.application.logs_buffer_max_records");
+
+    // todo not supported in SDK yet (this is strictly typed)


created open-telemetry/opentelemetry-java#7468

…anyways

...in/java/io/opentelemetry/javaagent/tooling/config/DeclarativeConfigEarlyInitAgentConfig.java

laurit · 2025-07-07T14:56:12Z

...nt-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/EarlyInitAgentConfig.java

- * io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties}) is initialized.
- */
-public final class EarlyInitAgentConfig {
+public interface EarlyInitAgentConfig {


This is some brave stuff. I suspect that you don't fully comprehend why the original code was written the way it is now and where the footgun lays.

shall I make a walkthrough in the SIG meeting?

shall I make a walkthrough in the SIG meeting?

I don't think it is necessary, it is not hard to see what your goal is.
If you look at how the EarlyInitAgentConfig is currently used you'll see that it is mostly about logging. The problem with logging is that currently it should be configured before anything is logged. You should check what will happen when you try to log anything before. Idk might not be too bad, could be that only the logger that was created before logging is configured will end up as a nop logger. Then would need to hope that nothing important is logged with that logger. The more code you need to run the harder it is to verify that that code does not use logging before it is configured. Even if you manage to verify that currently the declarative config code and all the libraries it uses doesn't use logging before it is configured you can't guarantee that it won't happen in a future version. Currently we have tried not to use logging before it is configured, you should try to figure out whether this is sustainable or maybe a different approach is needed. For example could buffer log messages before logging is configured.
Another issue is that we wish to install virtual fields into some jdk classes in

opentelemetry-java-instrumentation/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java

Line 230 in 9c2bea5

private static void installEarlyInstrumentation(

. For this we need to set up the transformations before these classes are loaded, otherwise virtual fields will fall back to a map which is undesirable. Reading the declarative config is going to pull in a bunch of classes which could load the classes we wish to transform. We may need to rearrange the code to install the early instrumentations before configuration is available to ensure that we can transform the classes before they are loaded.
There are some configuration options like

opentelemetry-java-instrumentation/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationConfiguration.java

Line 15 in 9c2bea5

config.getBoolean("otel.javaagent.experimental.field-injection.enabled", true);

that aren't really useful for end users, imo it is not important for these to be configurable from declarative config (if needed could consider removing the option).

shall I make a walkthrough in the SIG meeting?

I don't think it is necessary, it is not hard to see what your goal is. If you look at how the EarlyInitAgentConfig is currently used you'll see that it is mostly about logging.

so we have

logging

bytecode transformation

early return (if agent is disabled)

The motivation for my change is the early return, which is done differently in case of the config file.
Maybe this is not worth it - and we should go back to using sys properties for this.

that aren't really useful for end users

great point - can you list which properties are actually relevant for users from the list in #14132 ?

The motivation for my change is the early return, which is done differently in case of the config file.
Maybe this is not worth it - and we should go back to using sys properties for this.

It could be done, but it requires careful handling.

great point - can you list which properties are actually relevant for users from the list in #14132 ?

I think the following could be useful for endusers
otel.javaagent.add-thread-details
otel.javaagent.configuration-file
otel.javaagent.debug
otel.javaagent.enabled
otel.javaagent.exclude-class-loaders
otel.javaagent.exclude-classes
otel.javaagent.experimental.security-manager-support.enabled
otel.javaagent.experimental.thread-propagation-debugger.enabled
otel.javaagent.extensions
otel.javaagent.logging.application.logs-buffer-max-records
otel.javaagent.logging

not read from config file with current PR

otel.javaagent.experimental.security-manager-support.enabled

otel.javaagent.experimental.thread-propagation-debugger.enabled

would not be read from config file if we choose a simpler approach

otel.javaagent.extensions

otel.javaagent.logging.application.logs-buffer-max-records

otel.javaagent.logging

otel.javaagent.enabled

is read from config file either way

otel.javaagent.exclude-classes

otel.javaagent.exclude-class-loaders

otel.javaagent.add-thread-details

mix of all of the above

otel.javaagent.debug

disallowed with declarative config

otel.javaagent.configuration-file

zeitlinger · 2025-07-08T12:51:27Z

We may need to rearrange the code to install the early instrumentations before configuration is available to ensure that we can transform the classes before they are loaded.

This still leaves "otel.javaagent.enabled" - which promises not to touch anything if the value is false.

zeitlinger · 2025-07-08T13:03:50Z

The problem with logging is that currently it should be configured before anything is logged

https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java doesn't log for the method that we call - but jackson can throw exceptions (which I can buffer like ConfigurationFile)

zeitlinger · 2025-07-08T13:06:30Z

Reading the declarative config is going to pull in a bunch of classes which could load the classes we wish to transform

How can we find out if that's the case? E.g. a tool to compare loaded classes against what we want to transform.

zeitlinger · 2025-07-08T13:26:58Z

The problem with logging is that currently it should be configured before anything is logged

https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java doesn't log for the method that we call - but jackson can throw exceptions (which I can buffer like ConfigurationFile)

extracted ErrorBuffer: https://github.com/zeitlinger/opentelemetry-java-instrumentation/blob/eb8ee1978206d602db9e07b02e09b36e01d3cad3/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ErrorBuffer.java

zeitlinger requested a review from a team as a code owner July 4, 2025 16:56

zeitlinger marked this pull request as draft July 4, 2025 16:56

zeitlinger self-assigned this Jul 4, 2025

zeitlinger commented Jul 7, 2025

View reviewed changes

...in/java/io/opentelemetry/javaagent/tooling/config/DeclarativeConfigEarlyInitAgentConfig.java Show resolved Hide resolved

zeitlinger marked this pull request as ready for review July 7, 2025 08:14

zeitlinger commented Jul 7, 2025

View reviewed changes

zeitlinger added 22 commits July 7, 2025 13:37

use _ instead of - in declarative config

f8cdbe6

map "common-enabled"

52893c8

use yaml file

1ad5e8a

agent properties, part 1

c253f13

early init config

ecaadf7

early init config

69fc94f

don't pass model to create the SDK - we need to parse the file twice …

8fc9067

…anyways

map agent enabled

a98546d

map agent props

2a65f94

map agent props

41b674b

fix

f7b4fda

fix

813e53c

fix

bc0febe

fix

4ead4e2

map general props

53f132c

map agent props

304289a

map agent props

40fb7a6

create sdk from previously read config model

1050615

create sdk from previously read config model

7c7b1d5

map otel.javaagent.debug

0fbc4d4

format

44512ec

format

db00f64

zeitlinger added 6 commits July 7, 2025 13:37

set test exporters

c46cc0d

use extension classloader

6a3eb14

format

5ee22ce

inline method

156865c

set global config provider

91cb16f

add more tests

a36770e

zeitlinger force-pushed the declarative-config-underscore branch from ea7fdfd to a36770e Compare July 7, 2025 11:37

zeitlinger mentioned this pull request Jul 7, 2025

Spring starter declarative config #14062

Draft

laurit reviewed Jul 7, 2025

View reviewed changes

...in/java/io/opentelemetry/javaagent/tooling/config/DeclarativeConfigEarlyInitAgentConfig.java Outdated Show resolved Hide resolved

laurit reviewed Jul 7, 2025

View reviewed changes

cleanup

10e3108

zeitlinger added 2 commits July 8, 2025 15:21

buffer errors

c4562fc

avoid reflection

eb8ee19

zeitlinger added 2 commits July 8, 2025 15:36

avoid reflection

c370544

format

e4e0b67

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

declarative config: early init and property mapping #14184

declarative config: early init and property mapping #14184

zeitlinger commented Jul 4, 2025 •

edited

Loading

Uh oh!

Uh oh!

zeitlinger commented Jul 7, 2025 •

edited

Loading

Uh oh!

zeitlinger Jul 7, 2025

Uh oh!

zeitlinger Jul 7, 2025

Uh oh!

Uh oh!

laurit Jul 7, 2025

Uh oh!

zeitlinger Jul 7, 2025

Uh oh!

laurit Jul 7, 2025

Uh oh!

zeitlinger Jul 8, 2025

Uh oh!

laurit Jul 8, 2025

Uh oh!

zeitlinger Jul 8, 2025

Uh oh!

zeitlinger commented Jul 8, 2025

Uh oh!

zeitlinger commented Jul 8, 2025

Uh oh!

zeitlinger commented Jul 8, 2025

Uh oh!

zeitlinger commented Jul 8, 2025

Uh oh!

Uh oh!

	testing {
	suites {
	val declarativeConfigTest by registering(JvmTestSuite::class) {
	targets {
	all {
	testTask.configure {
	jvmArgs(
	"-Dotel.experimental.config.file=$projectDir/src/declarativeConfigTest/resources/declarative-config.yaml"
	)
	}
	}
	}
	}
	}
	}

declarative config: early init and property mapping #14184

Are you sure you want to change the base?

declarative config: early init and property mapping #14184

Conversation

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

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

zeitlinger commented Jul 8, 2025

Uh oh!

zeitlinger commented Jul 8, 2025

Uh oh!

zeitlinger commented Jul 8, 2025

Uh oh!

zeitlinger commented Jul 8, 2025

Uh oh!

Uh oh!

zeitlinger commented Jul 4, 2025 •

edited

Loading

zeitlinger commented Jul 7, 2025 •

edited

Loading