Skip to content

feat(telemetry): opt-out, isolated Sentry error telemetry for the SDK - #756

Open
GregHolmes wants to merge 4 commits into
mainfrom
gh/telemetry-observability
Open

feat(telemetry): opt-out, isolated Sentry error telemetry for the SDK#756
GregHolmes wants to merge 4 commits into
mainfrom
gh/telemetry-observability

Conversation

@GregHolmes

Copy link
Copy Markdown
Contributor

What

Adds an opt-out, isolated, error-only Sentry telemetry module (src/deepgram/telemetry/), wired into DeepgramClient / AsyncDeepgramClient via the existing telemetry_opt_out and telemetry_handler params. The goal is visibility into SDK failures (HTTP 5xx, transport/SDK errors) without touching customer data or the host app's own Sentry.

Design

  • Isolated from the host app. Never calls the global sentry_sdk.init(); builds a private Client with default_integrations=False, so we only ever capture SDK-originated errors and leave the customer's own Sentry setup untouched. This follows Sentry's own guidance for library authors.
  • Per-client, not global. Capture routes through a per-client TelemetrySink, so one client's opt-out or custom handler can't leak into another's.
  • No personal data. send_default_pii=False, include_local_variables=False, max_request_body_size="never", plus a before_send scrubber that strips request bodies, query strings, cookies, credential headers, server_name, user (incl. geo), and redacts the home-dir username from stack-frame paths (/Users/<name>/... -> /Users/<redacted>/...).
  • Custom handler fully replaces Sentry when supplied (nothing leaves the process).

Safety / posture

  • Off by default and inert. telemetry_opt_out defaults to True, _EMBEDDED_DSN is empty, and a DEEPGRAM_TELEMETRY_DISABLED kill-switch wins over everything. With no resolvable DSN this no-ops entirely, so merging this ships nothing live — arming is a separate, gated step.
  • Load-shedding / bounded queue behaviour is inherited from sentry-sdk's background worker (network post is off the request path).

Testing

tests/custom/test_telemetry.py covers opt-out resolution, the env kill-switch, PII/secret scrubbing, stack-frame path redaction, per-client isolation, and host-app isolation. pytest, ruff, and mypy all pass.

Add a hand-written SDK observability module that builds an isolated Sentry
client (never the global sentry_sdk.init()) wired into client.py via the
telemetry_opt_out / telemetry_handler constructor params. Adds the optional
sentry-sdk dependency and a 'telemetry' extra, freezes the module and its
test in .fernignore, and adds a regression test.
- Route capture through a per-client TelemetrySink instead of module globals,
  so one client's opt-out or handler no longer leaks to another (fixes the
  opt-out bypass and sticky-handler bugs)
- Stop hardcoding environment="production"; honour DEEPGRAM_TELEMETRY_ENVIRONMENT,
  otherwise leave it unset
- Make install_response_capture genuinely idempotent (guard against stacking
  duplicate response hooks)
- Correct the DeepgramClient telemetry_opt_out/telemetry_handler docstrings,
  which still described the params as no-ops
- Redact the home-dir username from stack-frame abs_path/filename in scrub_event
- Update and extend the telemetry tests
… in scrub

CI runs mypy without the optional telemetry extra installed, so the
TYPE_CHECKING import of sentry_sdk.types failed with import-not-found.
Add the same # type: ignore[import-not-found] already used on the other
sentry imports in client.py.

@paultill paultill left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please see https://deepgram.slack.com/archives/C07E6NG16SZ/p1786549158800029?thread_ts=1772209500.054649&cid=C07E6NG16SZ

Sounds good. I'll state what I am hoping for concisely:

  • For the first version, be as minimal as possible to keep complexity low. (Add more later once we de-risk the initial deployment of telemetry)
  • Either modify Luke's doc or write your own doc to describe the PR's design. It should focus more on what you value rather than repeating the technical details that appear in the PR. Shorter is better!
  • Be intentional about how you know that the new functionality in the SDK doesn't break client applications. This should be both testing (CC @Edward Rosen) and some design considerations such as bounded resource usage and making sure that errors posting telemetry are contained.

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.

2 participants