fix spring declarative config with env var substitution #15775
Conversation
github-actions
bot
added
the
test native
trask
left a comment
trask
left a comment
There was a problem hiding this comment.
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
| string_key_with_env_quoted: "${STRING_ENV:default_quoted_value}" | ||
| int_key: 42 | ||
| int_key_with_env: ${INT_ENV:100} | ||
| int_key_with_env_quoted: "${INT_ENV:200}" |
There was a problem hiding this comment.
should "${INT_ENV:200}" resolve to a string instead of an int?
e.g. see the examples at https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/data-model.md#environment-variable-substitution
or if we're aligning with Spring rules for mapping yaml data to types, can you link to those rules?
There was a problem hiding this comment.
Spring has this workflow:
- flatten YAML to flat properties
- resolve expressions
- inject and bind into application (e.g. ConfigurationProperties)
The problem is that the type info is lost when expressions are resolved - unlike the SDK YAML parser (as shown below).
I tried parsing the YAML file directly at first, but this breaks down when you use multiple YAML files (e.g. for a production profile), or when you use env var overrides, e.g. OTEL_INSTRUMENTATION_DEVELOPMENT_JAVA_FOO_INT_KEY = 43.
@laurit please double check if I missed something, as you helped with this part.
There was a problem hiding this comment.
Thinking a bit more about this I've found a solution that is only coerces environment variables: #15784
Let me know what you think.
2 participants
@trask the tests were lacking - so it seemed fine to remove SpringConfigProvider - but it turned out that it's needed.