Skip to content

Match environment variables for underscore keys in DefaultConfigProperties - #8951

Merged
jack-berg merged 1 commit into
open-telemetry:mainfrom
thswlsqls:fix/default-config-properties-env-var-underscore-keys
Nov 3, 2026
Merged

jack-berg merged 1 commit into
open-telemetry:mainfrom
thswlsqls:fix/default-config-properties-env-var-underscore-keys

Conversation

@thswlsqls

Copy link
Copy Markdown
Contributor

Related to #8932. Follow-up to #8933, which fixes the same mismatch in ConfigUtil.getString.

Description

  • DefaultConfigProperties stored environment variable keys with _ replaced by ., but kept _ for all other keys and lookups.
  • So OTEL_JAVA_EXPORTER_MEMORY_MODE was ignored: stored as otel.java.exporter.memory.mode, looked up as otel.java.exporter.memory_mode.
  • Normalize every stored and looked-up key the same way (lower case, - and _ replaced with .), and update the class Javadoc. Keys without _ behave as before.
  • All sources now write to the same key, so the documented priority holds: system properties over environment variables over defaultProperties, with withOverrides on top.
  • a_b and a.b now map to the same key, as environment variables already did.

Testing done

  • Added three ConfigPropertiesTest cases with OTEL_JAVA_EXPORTER_MEMORY_MODE=immutable_data set in the module's test task: env var only, env var over default, system property over env var.
  • The first two fail without the fix (null and "reusable_data").
  • ./gradlew :sdk-extensions:autoconfigure-spi:check passed (19 tests).
  • ./gradlew :sdk-extensions:autoconfigure:check :exporters:otlp:all:test passed (198 and 117 tests).

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-11-03 15:36 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.65%. Comparing base (4662b08) to head (53b050c).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8951      +/-   ##
============================================
+ Coverage     90.61%   90.65%   +0.04%     
- Complexity    10882    10889       +7     
============================================
  Files          1015     1015              
  Lines         29731    29731              
  Branches       3876     3876              
============================================
+ Hits          26942    26954      +12     
+ Misses         1872     1860      -12     
  Partials        917      917              

☔ View full report in Codecov by Harness.
📢 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.

@thswlsqls
thswlsqls marked this pull request as ready for review October 22, 2026 23:27
@thswlsqls
thswlsqls requested a review from a team as a code owner October 22, 2026 23:27

@jack-berg jack-berg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This does have some behavior changes as described in the table below. However I think its acceptable and the existing behavior should be classified as a bug.

With this PR, the summary of sys property / env var behavior is: Property names and lookup keys are case-insensitive and treat ., _, and - equivalently. Standard underscore-separated environment variables map to those same keys.

Environment variable System property Lookup key Before After
OTEL_JAVA_EXPORTER_MEMORY_MODE=immutable_data None otel.java.exporter.memory_mode null immutable_data
A_B=ENV None a_b null ENV
A_B=ENV None a.b ENV ENV
A_B=ENV None a-b ENV ENV
None -Da_b=SYS a_b SYS SYS
None -Da_b=SYS a.b null SYS
None -Da_b=SYS a-b null SYS
None -Da.b=SYS a_b null SYS
None -Da.b=SYS a.b SYS SYS
None -Da.b=SYS a-b SYS SYS
None -Da-b=SYS a_b null SYS
None -Da-b=SYS a.b SYS SYS
A_B=ENV -Da_b=SYS a_b SYS SYS
A_B=ENV -Da_b=SYS a.b ENV SYS
A_B=ENV -Da_b=SYS a-b ENV SYS
A_B=ENV -Da.b=SYS a_b null SYS
A_B=ENV -Da.b=SYS a.b SYS SYS

Heads up @open-telemetry/java-approvers. Will merge on 11/3 if no other feedback.

@jack-berg
jack-berg merged commit b56e7a1 into open-telemetry:main Nov 3, 2026
35 checks passed
@otelbot

otelbot Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution @thswlsqls! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

@thswlsqls
thswlsqls deleted the fix/default-config-properties-env-var-underscore-keys branch November 4, 2026 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants