Skip to content

opentelemetry-configuration: support slashes in the field name for declarative configuration#5396

Closed
emdneto wants to merge 3 commits into
open-telemetry:mainfrom
emdneto:emd5395
Closed

opentelemetry-configuration: support slashes in the field name for declarative configuration#5396
emdneto wants to merge 3 commits into
open-telemetry:mainfrom
emdneto:emd5395

Conversation

@emdneto

@emdneto emdneto commented Jul 6, 2026

Copy link
Copy Markdown
Member

Description

Fixes #5395

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

emdneto added 2 commits July 6, 2026 20:36
Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
@emdneto emdneto requested a review from a team as a code owner July 6, 2026 23:40
Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
@emdneto emdneto moved this to Ready for review in Python PR digest Jul 7, 2026
@emdneto emdneto added the config Issues and PRs related to implementing Declarative Config label Jul 7, 2026
@aabmass aabmass requested a review from Copilot July 7, 2026 16:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +109 to 113
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:

@xrmx xrmx Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WDYT on using a double underscore instead just to be a bit more wary of name collisions?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@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

@tammy-baylis-swi tammy-baylis-swi moved this from Ready for review to Approved PRs in Python PR digest Jul 7, 2026

@MikeGoldsmith MikeGoldsmith 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.

Awesome, thanks @emdneto 🎉

@xrmx

xrmx commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Closing as #5372 includes the very same change

@xrmx xrmx closed this Jul 8, 2026
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Python PR digest Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config Issues and PRs related to implementing Declarative Config

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

opentelemetry-configuration: declarative configuration does not work with slash in the field name

7 participants