Skip to content

refactor(telemetry): align metrics/pricing init with tracing pattern#1252

Open
ajbozarth wants to merge 1 commit into
generative-computing:mainfrom
ajbozarth:refactor/telemetry-init
Open

refactor(telemetry): align metrics/pricing init with tracing pattern#1252
ajbozarth wants to merge 1 commit into
generative-computing:mainfrom
ajbozarth:refactor/telemetry-init

Conversation

@ajbozarth

@ajbozarth ajbozarth commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Issue

Fixes #

Description

Brings mellea.telemetry.metrics and mellea.telemetry.pricing onto the same module-init pattern that mellea.telemetry.tracing adopted in #1181. Eliminates importlib.reload(mellea.telemetry.*) from every telemetry test (~40 sites) and consolidates the three drifted copies of _reset_tracing_state() that PR #1181 left behind.

No public API change. No production behavior change. Test side absorbed nearly all of the diff.

Why: after #1181, tracing.py was the only telemetry pillar with its init logic in a recallable _setup_tracing() function. metrics.py and pricing.py still captured env vars into module-level constants at the top of the module, which forced every test that toggled an env var to importlib.reload(mellea.telemetry.metrics) to re-execute the module body. Each reload re-runs imports and side effects — slow, noisy, and fragile.

Source-side changes:

  • metrics.py — Module-level env-read constants removed. The seven private _METRICS_* / _OTLP_METRICS_ENDPOINT / _SERVICE_NAME / _EXPORT_INTERVAL_MILLIS constants no longer exist; env-var reads live inside _setup_meter_provider(), matching tracing.py. The hot-path gate in every record_* function checks _meter is None directly (semantically equivalent to the old _METRICS_ENABLED check). Also fixes a parallel of the tracing-side regression caught in fix: reload module for telemetry testing so all tests can run #805: _meter is now bound off the freshly built provider rather than via OTel's global metrics.get_meter(), since set_meter_provider() is one-shot per process and the global meter would otherwise fall back to the original (now shutdown) provider after a reset.
  • pricing.py — Init logic wrapped in _setup_pricing(). Same shape as the others; no production behavior change.

Minor cleanup: the redundant if is_tracing_enabled(): guard at the module-load _setup_tracing() call site is dropped (the function early-returns on its own), and is_metrics_enabled() / is_tracing_enabled() now cache their result like is_pricing_enabled() already does.

Test-side changes:

  • New test/telemetry/conftest.py with shared reset_metrics_state(), reset_tracing_state(), reset_logging_state(), reset_pricing_state() helpers replacing all importlib.reload calls. New regression test test_meter_remains_functional_after_repeated_resets exercises _setup_metrics() across multiple reset cycles and asserts recordings still land in the active reader.
  • Tracing test reset DRY-up: three drifted per-file copies of _reset_tracing_state() consolidated. test_tracing.py and test_tracing_backend.py previously omitted the _tracer_provider.shutdown() call that test_tracing_plugins.py had — the consolidated helper does the shutdown for all three. Strictly safer cleanup; no perceptible slowdown (idle BatchSpanProcessor workers join in microseconds).
  • Test mocking layer change for _LITELLM_AVAILABLE and _OTEL_AVAILABLE: tests no longer manipulate sys.modules to fake import failure. They now use monkeypatch.setattr(...) directly on the module attribute. Same code paths exercised; module-load-time try: import ... blocks are no longer reverified per-test.
  • Test assertions in test_metrics.py shifted from cached env captures (_METRICS_CONSOLE is True, _OTLP_METRICS_ENDPOINT == "...", etc.) to behavior on the constructed MeterProvider (resource attributes, attached metric readers, mock-patched exporter call args). Strictly stronger — proves env values were both read and applied.
  • test/telemetry/__init__.py added so from test.telemetry.conftest import ... resolves under mypy. Same pattern test/plugins/__init__.py already uses.

What didn't change:

  • Public API: is_metrics_enabled(), is_tracing_enabled(), is_pricing_enabled(), get_otlp_log_handler(), all record_* and create_* functions — same signatures, same semantics.
  • Production behavior: no env var renames, no breaking changes, no warning text changes.
  • Documentation: no doc updates required.
  • Plugin registration: _plugins_registered semantics preserved (process-global, intentionally not reset by tests).

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code was added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)
$ uv run pytest test/telemetry/ -m "not qualitative"
211 passed, 17 deselected

2 e2e tests in test_metrics_backend.py::test_huggingface_token_metrics_integration fail on local — pre-existing huggingface.py:1119 torch tensor-indexing issue, unrelated to this PR. Not seen in -m "not e2e".
Opened #1253

Attribution

  • AI coding assistants used

Adding a new component, requirement, sampling strategy, or tool?

If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.

  • Component
  • Requirement
  • Sampling Strategy
  • Tool

NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.

Wraps init logic in metrics.py and pricing.py in callable `_setup_*()`
functions so tests can re-trigger init via a helper instead of
`importlib.reload`.

Drops the seven module-level env-read constants from metrics.py; env
vars are now read at call time inside `_setup_meter_provider()` and
hot-path record_* gates check `_meter is None`.

Adds test/telemetry/conftest.py with shared `reset_*_state()` helpers
that replace ~40 `importlib.reload(mellea.telemetry.*)` calls across
all four telemetry test suites. Consolidates three drifted copies of
`_reset_tracing_state()` left over from PR generative-computing#1181.

Assisted-by: Claude Code
Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
@ajbozarth ajbozarth requested a review from a team as a code owner June 10, 2026 21:06
@ajbozarth ajbozarth self-assigned this Jun 10, 2026
@github-actions github-actions Bot added the enhancement New feature or request label Jun 10, 2026
@ajbozarth ajbozarth requested a review from planetf1 June 10, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant