fix(telemetry): make ThreadingInstrumentor opt-in with env-var precedence#2167
Open
jpr5 wants to merge 1 commit into
Open
fix(telemetry): make ThreadingInstrumentor opt-in with env-var precedence#2167jpr5 wants to merge 1 commit into
jpr5 wants to merge 1 commit into
Conversation
yonib05
added
area-otel
maxmilian
added a commit
to maxmilian/harness-sdk
that referenced
this pull request
Jun 24, 2026
…_DISABLED / enabled (strands-agents#1059) StrandsTelemetry had no way to turn instrumentation off, so applications that already run their own OpenTelemetry setup still got Strands' tracer/meter providers registered globally and its spans/metrics emitted — even when they didn't want them (see strands-agents#1059, e.g. metrics that a downstream CloudWatch pipeline can't flush). Add an `enabled` switch to StrandsTelemetry that defaults to honoring the OTel-standard `OTEL_SDK_DISABLED=true` environment variable, with an explicit `StrandsTelemetry(enabled=False)` override for programmatic control. When disabled, no global tracer/meter provider is registered and the `setup_*` methods are no-ops, leaving the host application's OpenTelemetry setup untouched. The default (enabled) path is unchanged, so this is backward compatible. Scoped to `telemetry/config.py` (the location @zastrowm suggested on the issue); the separate ThreadingInstrumentor opt-in work in strands-agents#2167 is untouched.
9 tasks
lizradway
reviewed
Jun 28, 2026
strands>=1.35.0 unconditionally calls ThreadingInstrumentor().instrument() in
Tracer.__init__, monkey-patching concurrent.futures.ThreadPoolExecutor.submit
process-wide, ignoring OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, double-wrapping
when another OTel distro already instrumented, and crashing the host if
instrument() raises.
This is additive and non-breaking: threading instrumentation stays ON by
default (unchanged behavior). It adds supported opt-out/opt-in levers and
hardens the call. Resolver precedence (highest->lowest):
1. OTEL_PYTHON_DISABLED_INSTRUMENTATIONS containing 'threading' -> disabled
(matches OpenTelemetry auto-loader semantics).
2. Explicit Tracer(instrument_threading=True|False) kwarg.
3. STRANDS_INSTRUMENT_THREADING env ('1/true/yes/on' enable; '0/false/no/off'
disable; unrecognized non-empty -> WARNING + default).
4. Default: ENABLED.
Also: an idempotency guard (reads _is_instrumented_by_opentelemetry to avoid
stacking wrappers when the host already instrumented) and graceful-failure
handling (instrument() exceptions are caught and logged -- ERROR when
explicitly requested, WARNING when auto-enabled -- so telemetry never crashes
the host).
Tests run in isolated subprocesses (BaseInstrumentor is a process-wide
singleton): on-by-default, env opt-in/opt-out incl. 'off', kwarg opt-in/out,
disable-env precedence over kwarg, idempotency, graceful failure, ERROR-vs-
WARNING severity, and warn-on-unrecognized.
Default->off is deferred to v2 (breaking).
jpr5
force-pushed
the
fix/threading-instrumentor-opt-in
branch
from
365c290 to
9498dc5
Compare
Author
|
Makes sense — keeping the default on so this stays additive. I left in the kwarg, the env var, and the LMK what you think @lizradway 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
strands>=1.35.0 unconditionally calls
ThreadingInstrumentor().instrument()inTracer.__init__, monkey-patchingconcurrent.futures.ThreadPoolExecutor.submitprocess-wide for every strands user. It ignores the standardOTEL_PYTHON_DISABLED_INSTRUMENTATIONS=threadingopt-out, double-wraps when another OTel distro (Azure Monitor, opentelemetry-distro, AWS OTel) already instrumented threading, and crashes the host ifinstrument()raises.This PR is additive and non-breaking: threading instrumentation stays ON by default (unchanged behavior). It adds supported opt-out/opt-in levers and hardens the call path.
Resolver precedence (highest → lowest):
OTEL_PYTHON_DISABLED_INSTRUMENTATIONScontainingthreading→ always disabled (matches OpenTelemetry auto-loader semantics).Tracer(instrument_threading=True|False)kwarg.STRANDS_INSTRUMENT_THREADINGenv var (1/true/yes/onenable;0/false/no/offdisable; unrecognized non-empty → WARNING + default).Also adds:
_is_instrumented_by_opentelemetryto avoid stacking wrappers when the host already instrumented threading), andinstrument()exceptions are caught and logged (ERROR when explicitly requested, WARNING when auto-enabled) so telemetry failure never crashes the host.Tests run in isolated subprocesses (
BaseInstrumentoris a process-wide singleton): on-by-default, env opt-in/opt-out (incl.off), kwarg opt-in/out, disable-env precedence over kwarg, idempotency, graceful failure, ERROR-vs-WARNING severity, and warn-on-unrecognized.Flipping the default to off is a breaking change deferred to v2 (tracked on this thread).