opentelemetry-configuration: support slashes in the field name for declarative configuration#5396
opentelemetry-configuration: support slashes in the field name for declarative configuration#5396emdneto wants to merge 3 commits into
Conversation
Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request fixes declarative configuration parsing for OpenTelemetry Configuration schema field names that include / (e.g. detection/development) by normalizing those keys to valid Python dataclass field names.
Changes:
- Normalize configuration mapping keys by replacing
/with_during dict-to-dataclass conversion. - Add unit tests to verify slash-to-underscore mapping for both known dataclass fields and additional-properties capture.
- Add a changelog entry describing the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_conversion.py | Normalizes config keys containing / so they can map onto Python dataclass fields. |
| opentelemetry-sdk/tests/_configuration/test_conversion.py | Adds regression tests ensuring slash keys are handled correctly in conversion. |
| .changelog/5396.fixed | Records the user-visible fix in the changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| normalized_key = key.replace("/", "_") | ||
| if normalized_key in known_fields: | ||
| type_hint = hints.get(normalized_key) | ||
| kwargs[normalized_key] = _convert_value(value, type_hint) | ||
| else: |
There was a problem hiding this comment.
WDYT on using a double underscore instead just to be a bit more wary of name collisions?
There was a problem hiding this comment.
@xrmx The generated dataclass fields currently use single underscores, so if we do that, we need to modify the schema generation to represent the known_fields with double underscores as well. I can implement the collision detection to raise a TypeError in case of collisions
MikeGoldsmith
left a comment
There was a problem hiding this comment.
Awesome, thanks @emdneto 🎉
|
Closing as #5372 includes the very same change |
Description
Fixes #5395
Type of change
Please delete options that are not relevant.