Skip to content

refactor: extract OpenTelemetryServiceFieldsConfig mixin#94

Merged
lesnik512 merged 1 commit into
mainfrom
fix/des-2-otel-fields-mixin
May 31, 2026
Merged

refactor: extract OpenTelemetryServiceFieldsConfig mixin#94
lesnik512 merged 1 commit into
mainfrom
fix/des-2-otel-fields-mixin

Conversation

@lesnik512
Copy link
Copy Markdown
Member

Summary

opentelemetry_service_name and opentelemetry_namespace were declared identically on both OpentelemetryConfig and PyroscopeConfig. In the four framework configs (Free, FastAPI, Litestar, FastStream) that inherit from both parents, Python's MRO happened to pick one declaration; the fact that defaults matched is what kept behavior consistent. Without the mixin, drifting defaults on one side would silently misbehave on the framework configs.

Extract OpenTelemetryServiceFieldsConfig(BaseConfig) — a tiny mixin declaring just those two fields. Both OpentelemetryConfig and PyroscopeConfig now inherit from it (no longer from BaseConfig directly). The duplicate declarations are removed.

PyroscopeConfig's standalone use case (without OpentelemetryConfig in the MRO) is preserved — exercised by test_pyroscope_standalone_config_accepts_otel_fields.

Closes DES-2 from an internal audit.

Test plan

  • just test -- tests/instruments/test_opentelemetry_instrument.py tests/instruments/test_pyroscope_instrument.py -v — pass.
  • just test — full suite 89/89.
  • just lint — clean.
  • Reviewer: confirm the new dependency direction (pyroscope_instrumentopentelemetry_instrument) is acceptable. Code review confirmed: no circular import — opentelemetry_instrument imports the pyroscope package, not pyroscope_instrument.

Notes for reviewer

The MRO for FreeBootstrapperConfig(LoggingConfig, OpentelemetryConfig, PyroscopeConfig, SentryConfig) resolves as:
FreeBootstrapperConfig → LoggingConfig → OpentelemetryConfig → PyroscopeConfig → OpenTelemetryServiceFieldsConfig → SentryConfig → BaseConfig → object

OpenTelemetryServiceFieldsConfig appears exactly once via C3 linearization — both children share the same ancestor.

Cosmetic future-work note: OpentelemetryConfig (lowercase t) sits next to the new OpenTelemetryServiceFieldsConfig (uppercase T) — same module, different casing. Worth a rename + deprecation alias in a follow-up PR. Out of scope here.

🤖 Generated with Claude Code

opentelemetry_service_name and opentelemetry_namespace were declared
identically on both OpentelemetryConfig and PyroscopeConfig. In the four
framework configs (Free, FastAPI, Litestar, FastStream) that inherit
from both parents, Python's MRO happened to pick one declaration; the
fact that defaults matched is what kept behavior consistent. Without
the mixin, drifting defaults on one side would silently misbehave on
the framework configs.

Extract OpenTelemetryServiceFieldsConfig(BaseConfig) — a tiny mixin
declaring just those two fields. Both OpentelemetryConfig and
PyroscopeConfig now inherit from it (no longer from BaseConfig
directly). The duplicate declarations are removed.

PyroscopeConfig's standalone use case (without OpentelemetryConfig in
the MRO) is preserved — exercised by
test_pyroscope_standalone_config_accepts_otel_fields.

Closes DES-2 from the audit.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
..._bootstrap/instruments/opentelemetry_instrument.py 100.00% <100.00%> (ø)
lite_bootstrap/instruments/pyroscope_instrument.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lesnik512 lesnik512 self-assigned this May 31, 2026
@lesnik512 lesnik512 merged commit 62e408e into main May 31, 2026
8 checks passed
@lesnik512 lesnik512 deleted the fix/des-2-otel-fields-mixin branch May 31, 2026 21:56
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.

1 participant