Skip to content

feat: add adaptive sampling to python sdk#90

Open
jy-tan wants to merge 8 commits intomainfrom
adaptive-sampling
Open

feat: add adaptive sampling to python sdk#90
jy-tan wants to merge 8 commits intomainfrom
adaptive-sampling

Conversation

@jy-tan
Copy link
Copy Markdown
Contributor

@jy-tan jy-tan commented Apr 10, 2026

Summary

Add the Python half of the adaptive sampling rollout so the SDK can reduce recording pressure based on local queue, exporter, and memory signals without sacrificing pre-app-start capture or backward compatibility with existing sampling configuration.

Changes

  • Parse the new nested recording.sampling config while keeping legacy sampling_rate behavior backwards compatible
  • Add an adaptive sampling controller and health loop driven by queue fill, dropped spans, exporter failures and circuit state, and memory pressure
  • Suppress child span creation when inbound requests are skipped by admission decisions while continuing to always record pre-app-start traffic
  • Standardize the canonical sampling env var on TUSK_RECORDING_SAMPLING_RATE while continuing to accept TUSK_SAMPLING_RATE as a backward-compatible alias
  • Retain compatibility for older _determine_sampling_rate call sites and expand unit coverage for adaptive sampling, config loading, mode handling, env-var precedence, and no-record span suppression
  • Add benchmark log analysis tooling plus related ignore rules to support tuning sampling-rate performance tradeoffs

Copy link
Copy Markdown
Contributor

@tusk-dev tusk-dev bot left a comment

Choose a reason for hiding this comment

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

Found 5 issues

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 16 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="drift/instrumentation/wsgi/handler.py">

<violation number="1" location="drift/instrumentation/wsgi/handler.py:229">
P2: suppress_recording ends before the WSGI response iterator is consumed, so child spans created during streaming can still be recorded for skipped requests. Wrap the returned iterable so suppression stays active through iteration.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="drift/instrumentation/wsgi/handler.py">

<violation number="1" location="drift/instrumentation/wsgi/handler.py:253">
P1: Skipped-request suppression no longer covers the initial WSGI app invocation, so eager app execution can still emit spans before the response iterable wrapper takes effect.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Copy link
Copy Markdown
Contributor

@tusk-dev tusk-dev bot left a comment

Choose a reason for hiding this comment

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

Found 2 issues

Copy link
Copy Markdown
Contributor

@tusk-dev tusk-dev bot left a comment

Choose a reason for hiding this comment

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

Found 2 issues

Copy link
Copy Markdown
Contributor

@tusk-dev tusk-dev bot left a comment

Choose a reason for hiding this comment

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

Found 3 issues

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