ENG-9166: suggestions for otel 3/3

Merged
MA
2mo ago
+410-228

Review follow-ups for #6901 (browser tracing through OtelPlugin), one finding per commit. OtelPlugin stays browser-only: the backend needs no plugin, and the plugin never touches reflex_base.otel.

What changes

Browser tracing

  • endpoint is required for exporting. With endpoint=None the plugin installs no exporter and an always-off sampler, and warns at compile time, so nothing is posted anywhere by default.
  • traceparent is injected only for sampled browser spans, so an unsampled request falls back to the backend's own root sampling instead of forcing a not-sampled decision on it.
  • File uploads carry a traceparent header, so the upload handler nests under the browser's upload span.
  • The browser socket span is PRODUCER, paired with the backend's CONSUMER event span.
  • The plugin's patches anchor on both entry roots and tolerate whitespace in the vite config, and the frontend hooks are optional-chained so an app without the plugin has no dangling calls.

Packaging

  • reflex_otel.ReflexInstrumentor is loaded lazily through __getattr__, so importing OtelPlugin in rxconfig.py does not pull in the instrumentor and opentelemetry. The instrumentor now lives in reflex_otel/instrumentor.py and the entry point points there.
  • The reflex-base floor (>= 0.9.10.post3.dev0) and the dropped runtime dependency check mirror #7069 on the moved module.

Docs and tests

  • The observability page describes the fixed behaviour: required endpoint, sample-rate semantics, uploads, guard-free setup, CONSUMER kind, a privacy section, semantic conventions and exclusions.
  • The browser module header and the stack's 6227.feature.md fragments for reflex-otel and reflex-base say what actually happens: an explicit endpoint, a traceparent on sampled sends only, and the upload helper's hook.
  • An end-to-end test compiles a real app with OtelPlugin and checks the injected entry, the vite config and env.json.

Rebase note

This branch is based on the 3/3 branch as it stands, so reflex_otel/instrumentor.py holds the pre-#7069 body of the instrumentor. Land #7069 first, then rebase this branch on the updated 2/3 and 3/3 bases: the __init__.py conflict resolves by keeping the move, and the #7069 changes to the instrumentor body (semantic-conventions opt-in, OTEL_SDK_DISABLED, token redaction, guard-free instrument(), environment-driven SDK setup) need to be carried into instrumentor.py. The floor and dependency-check commits here already match. The docs commit describes behaviour that #7069 introduces, which is why the review threads about environment-driven setup, session.id hashing and the CONSUMER kind point at #7069.

Review notes

  • exclude_spans exists at the ASGI instrumentation floor (0.49b0), and check_min_deps.py reflex-otel type-checks the call against that floor.
  • News fragments: this PR merges into the unreleased feature branch; the stack's PRs to main carry the 6227.feature.md fragments, kept truthful here.

Follow-up work (out of scope here)

  • Docs, metrics section: the Prometheus pull exporter cannot serve multi-worker reflex run --env prod, since only one worker binds the scrape port. Steer multi-worker deployments to OTLP push or the collector's Prometheus receiver.
  • Docs, once #7069 lands and this branch is rebased: re-read the setup section against the merged instrumentor.py so the page and the code cannot drift again.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update (included)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them? (each commit message carries its rationale)
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally? (full unit suite on this branch)

šŸ¤– Generated with Claude Code

https://claude.ai/code/session_01AsnJbeBU5w6piuxSJoz9gy