Skip to content

fix(config): align additional_properties typing#5286

Open
hariharan077 wants to merge 3 commits into
open-telemetry:mainfrom
hariharan077:fix-config-additional-properties-typing
Open

fix(config): align additional_properties typing#5286
hariharan077 wants to merge 3 commits into
open-telemetry:mainfrom
hariharan077:fix-config-additional-properties-typing

Conversation

@hariharan077

@hariharan077 hariharan077 commented Jun 10, 2026

Copy link
Copy Markdown

Description

Fixes #5268.

Declare generated config model additional_properties attributes as instance dataclass fields instead of ClassVars, matching the runtime behavior of @_additional_properties and the _ComponentConfig protocol.

This also skips non-init dataclass fields while resolving configured resource detectors, so the generated additional_properties field is not treated as a detector name.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Reproduced the strict pyright failure locally with a temporary SpanExporter/_resolve_component repro before the fix
  • Confirmed the same strict pyright repro passes after the fix with 0 errors, 0 warnings, 0 informations
  • .tox/py310-test-opentelemetry-sdk/bin/python -m pytest opentelemetry-sdk/tests/_configuration/test_common.py opentelemetry-sdk/tests/_configuration/test_resource.py -q
  • uv run tox -r -e typecheck
  • uv run tox -r -e lint-opentelemetry-sdk
  • uv run ruff check opentelemetry-sdk/src/opentelemetry/sdk/_configuration/models.py opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_common.py opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_resource.py opentelemetry-sdk/tests/_configuration/test_common.py
  • uv run ruff format --check opentelemetry-sdk/src/opentelemetry/sdk/_configuration/models.py opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_common.py opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_resource.py opentelemetry-sdk/tests/_configuration/test_common.py
  • git diff --check

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Changelog and codegen README documentation are included for the additional_properties field design.

@hariharan077 hariharan077 requested a review from a team as a code owner June 10, 2026 17:13
Declare generated additional_properties fields as instance dataclass fields, matching the @_additional_properties decorator runtime behavior and _ComponentConfig protocol.

Skip non-init dataclass fields when resolving configured resource detectors so the new generated field is not treated as a detector.

Assisted-by: ChatGPT
@hariharan077 hariharan077 force-pushed the fix-config-additional-properties-typing branch from 6b20701 to 8eb46b0 Compare June 10, 2026 17:14

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

Thanks @hariharan077 - sorry for the slow review.

There's a couple of things that need fixing:

  • We're still importing typing.ClassVar - I think we should be using dataclasses.field instead
  • The README.md needs updating with the new design, the field is now an init=False dataclass field with default_factory=dict and the @_additional_properties decorator assigns the extras in the constructor
  • You need to add a changelog fragment, something like .changelog/5286.fixed

@github-project-automation github-project-automation Bot moved this to Reviewed PRs that need fixes in Python PR digest Jun 24, 2026
@MikeGoldsmith MikeGoldsmith self-assigned this Jun 24, 2026
@MikeGoldsmith MikeGoldsmith added the config Issues and PRs related to implementing Declarative Config label Jun 24, 2026
Document that generated config models use an init=False dataclass field for additional_properties and add the changelog fragment requested in review.

Assisted-by: GPT-5 Codex
@hariharan077

Copy link
Copy Markdown
Author

Addressed the review comments in ebb349e: updated the codegen README for the init=False/default_factory additional_properties field, added .changelog/5286.fixed, and refreshed the PR checklist. Local focused tests, SDK lint, ruff, and towncrier draft all pass.

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: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

refactor(config): review additional_properties typing for stricter type checks

2 participants