Skip to content

fix spring declarative config with env var substitution#15775

Merged
trask merged 25 commits intoopen-telemetry:mainfrom
zeitlinger:spring-config-test
Feb 23, 2026
Merged

fix spring declarative config with env var substitution#15775
trask merged 25 commits intoopen-telemetry:mainfrom
zeitlinger:spring-config-test

Conversation

@zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Jan 6, 2026 •
edited
Loading

Reverts the "simplify" approach (commit 7fc9b04808) that replaced SpringDeclarativeConfigProperties + SpringConfigProvider with CoercingDeclarativeConfigProperties wrapping YamlDeclarativeConfigProperties.

The wrapper approach triggers noisy WARN logs from YamlDeclarativeConfigProperties when values are stored as strings (which is the case with Spring property resolution).

Reverting to the original approach avoids this issue. The upstream fix can be revisited via open-telemetry/opentelemetry-java#8101.

@zeitlinger zeitlinger self-assigned this Jan 6, 2026
@zeitlinger zeitlinger requested a review from a team as a code owner January 6, 2026 14:36
@github-actions github-actions bot added the test native This label can be applied to PRs to trigger them to run native tests label Jan 6, 2026
trask
trask reviewed Jan 7, 2026
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if we can find a way to map to typed data based on the yaml content only instead of dynamically coercing based on whether getInt or getString happens to be called by the user

@zeitlinger zeitlinger mentioned this pull request Jan 7, 2026
Closed
@trask trask added this to the v2.24.0 milestone Jan 9, 2026
@zeitlinger zeitlinger moved this to Awaiting Review in Declarative Configuration: Java Jan 12, 2026
@zeitlinger
Copy link
Member Author

zeitlinger commented Jan 15, 2026

@trask please have a look

@trask trask removed this from the v2.24.0 milestone Jan 16, 2026
@zeitlinger
Copy link
Member Author

zeitlinger commented Jan 27, 2026

@trask please have a look

@zeitlinger zeitlinger mentioned this pull request Feb 12, 2026
Merged
@trask trask mentioned this pull request Feb 14, 2026
Merged
trask
trask reviewed Feb 14, 2026
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sent a possible simplification: zeitlinger#8

zeitlinger and others added 4 commits February 17, 2026 12:36
@zeitlinger
pr review
643c34b 
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@trask @zeitlinger
simplify
7fc9b04
@zeitlinger
inline SpringConfigProvider and address review comments
71190a4 
- Delete SpringConfigProvider, use lambda for ConfigProvider since it's
  a functional interface
- Make CoercingDeclarativeConfigProperties.wrap() handle null
- Merge duplicate withType<Test>().configureEach blocks in build.gradle.kts
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Fix WARN from YamlDeclarativeConfigProperties for string-typed values
c8b74c7 
In CoercingDeclarativeConfigProperties, try getString() first before
the typed getter (getBoolean, getInt, etc). This avoids triggering the
warning in YamlDeclarativeConfigProperties when a value is stored as
String instead of its native type.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Copy link
Member Author

zeitlinger commented Feb 18, 2026

CI fix

The OtelSpringStarterSmokeTest > restClient() failure was caused by YamlDeclarativeConfigProperties logging a WARN for emit_experimental_telemetry/development — it found a String where it expected Boolean.

Root cause: environment.getProperty() returns all values as String, so YAML true becomes "true". CoercingDeclarativeConfigProperties.getBoolean() was calling delegate.getBoolean() first, which triggered the warning in YamlDeclarativeConfigProperties before returning null. The coercing wrapper then correctly fell through to getString() + parse, but the warning was already emitted.

Fix: Flip the order in CoercingDeclarativeConfigProperties — try delegate.getString() first, and only fall back to the typed getter when the value isn't a string. This avoids triggering the warning while still handling both string and natively-typed values.

trask
trask reviewed Feb 18, 2026
View reviewed changes
@trask trask mentioned this pull request Feb 18, 2026
Open
@zeitlinger
Address review comments: cache wrapped config, remove unused method, …
c73e197 
…simplify getString

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Revert simplify approach: restore SpringDeclarativeConfigProperties +…
037d381 
… SpringConfigProvider

The CoercingDeclarativeConfigProperties wrapper triggers noisy WARN logs
from YamlDeclarativeConfigProperties when values are stored as strings.
Revert to the original approach of SpringDeclarativeConfigProperties +
SpringConfigProvider which handles type coercion directly.

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Simplify type coercion: only coerce from String
1abe589 
Address review: remove cross-type conversions (Long->Integer,
Integer->Long, Float->Double, Boolean native) and only coerce
from String, since that's the only case Spring produces.

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
trask
trask reviewed Feb 20, 2026
View reviewed changes
@zeitlinger
Address review comments: type safety, error handling, and component l…
6513c0b 
…oader passthrough

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Address review: extract shared componentLoader var, handle native typ…
a99a2c4 
…es in coercion

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Copy link
Member Author

zeitlinger commented Feb 20, 2026

Addressed both review comments:

  1. Extract var — extracted componentLoader variable shared across both calls in OpenTelemetryAutoConfiguration.
  2. Handle native types — added instanceof checks for Boolean, Long, Double in the coercion methods. Also added Integer→Long conversion in longOrNull since YAML parsers produce Integer for small numbers.

zeitlinger and others added 4 commits February 20, 2026 19:45
@zeitlinger
spotlessApply
8bea13a 
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Merge branch 'main' into spring-config-test
a76e78f
@trask
add comment
f70fb87
@trask
spotless
08b6c66
@trask trask requested a review from Copilot February 23, 2026 19:20
Copilot AI reviewed Feb 23, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reverts the prior declarative-config “wrapper” approach for Spring Boot and restores a Spring-specific DeclarativeConfigProperties implementation to avoid noisy WARN logs when Spring resolves values as strings (notably with env var substitution).

Changes:

  • Add Spring-specific DeclarativeConfigProperties + ConfigProvider to coerce scalar types without triggering YAML-type warnings.
  • Adjust Spring embedded-config extraction to treat resolved Spring properties as String values and convert them into an OpenTelemetryConfigurationModel.
  • Expand declarative-config tests (including env var substitution/overrides) and update Gradle JVM args to support env var mutation in tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
instrumentation/spring/spring-boot-autoconfigure/src/testDeclarativeConfig/resources/application.yaml Expands declarative-config sample properties to cover scalar types and env var substitution (quoted/unquoted).
instrumentation/spring/spring-boot-autoconfigure/src/testDeclarativeConfig/java/io/opentelemetry/instrumentation/spring/autoconfigure/DeclarativeConfigTest.java Updates tests to assert typed access/coercion and validates env var overrides in both Spring-style and Otel-style forms.
instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/EmbeddedConfigFileTest.java Aligns tests with the flat-props map now being Map<String, String>.
instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/SpringDeclarativeConfigProperties.java Introduces Spring-specific declarative properties implementation with scalar coercion logic.
instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/SpringConfigProvider.java Adds Spring-specific ConfigProvider that feeds the coerced declarative properties to SpringOpenTelemetrySdk.
instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfiguration.java Wires declarative-config startup to return SpringOpenTelemetrySdk backed by the new Spring config provider.
instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/EmbeddedConfigFile.java Changes extraction to collect resolved Spring properties as strings and converts them into the OTel configuration model.
instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts Adds an additional --add-opens needed for env var mutation via JUnit Pioneer on newer JDKs.

trask
trask approved these changes Feb 23, 2026
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@trask trask enabled auto-merge (squash) February 23, 2026 22:12
@trask trask merged commit f7cba3b into open-telemetry:main Feb 23, 2026
165 of 167 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Declarative Configuration: Java Feb 23, 2026
@zeitlinger zeitlinger deleted the spring-config-test branch February 24, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@trask trask trask approved these changes

Assignees

@zeitlinger zeitlinger

Labels

test native This label can be applied to PRs to trigger them to run native tests

Projects

Declarative Configuration: Java
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zeitlinger @trask