Skip to content

fix(config): stop test runs exporting traces - #905

Closed
LucStr wants to merge 1 commit into
mainfrom
fix(config)/stop-test-runs-exporting-traces
Closed

LucStr wants to merge 1 commit into
mainfrom
fix(config)/stop-test-runs-exporting-traces

Conversation

@LucStr

@LucStr LucStr commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Problem

Most Rapidata.Python.SDK error spans in prod come from our own test runs: ~94% of the 1,665 SDK error spans in the last 7 days carry MagicMock, job-1 / My Job fixtures, or runner/vessel kernels. They hide real customer errors. The versions sending them (3.25.5–3.25.7) already include #867, which disabled OTLP under pytest.

Root cause: LoggingConfig.__init__ called _notify_handlers(), so any LoggingConfig instance reconfigured the global tracer and logger. test_explicit_true_overrides_the_pytest_default builds LoggingConfig(enable_otlp=True), and every test after it exported to otlp-sdk.rapidata.ai.

Fix

  • Drop the notify from LoggingConfig.__init__; rapidata_config.py notifies once for the global rapidata_config.logging. Attribute updates on the global (the documented API, rapidata_config.logging.x = …) still propagate through __setattr__.
  • tests/conftest.py: an autouse fixture resets enable_otlp after each test, so no single test can leak tracing into the rest of the run.
  • Regression test: a standalone LoggingConfig(enable_otlp=True) leaves the global tracer disabled.

Verification

  • Full suite: 290 passed.
  • A pytest_sessionfinish probe of the global tracer: on main it ends enabled=True initialized=True; with this change it ends enabled=False initialized=False.
  • pyright src/rapidata/rapidata_client/config: 0 errors.

🔗 Session: https://poseidon.rapidata.internal/chat/node-84f2b825850d

🤖 Generated with Claude Code

@LucStr

LucStr commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

@LinoGiger can you please look into why this is constantly failing and either close or merge this pr

@LucStr
LucStr marked this pull request as ready for review September 25, 2026 16:45
@LucStr
LucStr requested a review from LinoGiger as a code owner September 25, 2026 16:45
@RapidPoseidon
RapidPoseidon force-pushed the fix(config)/stop-test-runs-exporting-traces branch from 80dfdb8 to 45a9ea1 Compare September 29, 2026 08:59
@github-actions

Copy link
Copy Markdown

⚠ Agent skill not updated

This PR does not modify src/rapidata/_skill/. That directory is the agent skill that ships in every SDK release and that coding agents read through python -m rapidata skill.

Before merging, pick one:

  1. The change affects what an agent needs to know (new or renamed API, changed parameter, default or result field, new gotcha): update src/rapidata/_skill/SKILL.md (or a companion guide next to it) in this PR.
  2. Nothing the skill documents changed: a reviewer applies the skill-unchanged-approved label. New commits remove the label again.

@RapidPoseidon

Copy link
Copy Markdown
Contributor

Status after rebasing onto main (conflict in tests/conftest.py with #911 resolved by keeping both fixtures): build, type-check and test pass (325 tests). A session-end probe confirms the global tracer ends the run enabled=False.

The only red check is the new Agent Skill gate from #911. It isn't a code failure: it fails for any PR that leaves src/rapidata/_skill/ untouched, until a reviewer adds skill-unchanged-approved. The skill doesn't need editing for this PR. reference.md documents mutating the global config (rapidata_config.logging.enable_otlp = …, RAPIDATA_DISABLE_OTLP=1), and that behaves exactly as before. This PR only stops a standalone LoggingConfig(...) from reconfiguring the global tracer, and the skill never mentions that.

→ Reviewer: please add skill-unchanged-approved if you agree, then approve.

Constructing any LoggingConfig pushed its settings to the global tracer and
logger, so a standalone LoggingConfig(enable_otlp=True) re-enabled OTLP for
the whole process. Only the global rapidata_config.logging now notifies the
handlers; the conftest resync from #913 stays as a test-side backstop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: luca@rapidata.ai <25279790+LucStr@users.noreply.github.com>
@RapidPoseidon
RapidPoseidon force-pushed the fix(config)/stop-test-runs-exporting-traces branch from 45a9ea1 to 211285d Compare September 29, 2026 09:27
@RapidPoseidon

Copy link
Copy Markdown
Contributor

Rebased again, this time onto #913. #913 added a conftest.py fixture that re-applies the global config after each test, which overlaps with the reset fixture this PR had, so I took main's fixture and dropped mine. Its comment now says what it does, since after this PR a standalone LoggingConfig no longer changes the global tracer.

The PR is now the source fix plus its regression test: only rapidata_config.logging notifies the tracer/logger handlers. #913 guards the test suite; this PR stops any process that builds its own LoggingConfig(enable_otlp=True) from turning on export for the whole process. 325 tests pass, and the tracer ends the run enabled=False. The Agent Skill gate still needs skill-unchanged-approved (see the comment above).

@LinoGiger LinoGiger closed this Sep 29, 2026
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.

3 participants