Stop reporting success when the SDK is exporting nothing - #84
Merged
Conversation
This SDK told a production service it was fine while it emitted nothing for
months. `bootstrap()` returned `installed: true` whenever it wasn't explicitly
disabled — including the case where NO endpoint was configured, so no OTel SDK
was installed and every span, metric and log went nowhere. The no-endpoint
branch was completely silent; it warned about missing AUTH but not about the far
more consequential missing DESTINATION.
A test enshrined it:
let result = build(env).await;
assert!(result.installed);
assert!(result.otel.is_none()); // installed=true, exporter=None
How it played out: smooai chat-ws — the service running every LLM agent — set
only OTEL_EXPORTER_OTLP_ENDPOINT, so this SDK installed nothing while the
operator's own gRPC exporter aimed at an endpoint that speaks authenticated
HTTP/JSON. Production has 76 gen_ai rows, all from one other service, none from
chat-ws, ever. Nobody noticed because everything reported healthy.
Three changes:
1. `BootstrapResult::exporting` — whether an OTLP exporter was actually
installed. `installed` keeps its old meaning (bootstrap ran) and now says so
honestly in its doc comment instead of implying more.
2. The no-endpoint branch WARNS, names the variable to set, and offers
SMOOAI_OBSERVABILITY_DISABLED=true as the way to make the silence deliberate.
An intentional no-op should be declared, not inferred from absence.
3. The test that asserted the misleading shape now asserts `!exporting`, and a
new test asserts `exporting == true` when an endpoint IS set. Both halves
matter — with only one, hard-coding either value passes.
The general lesson, which is why this belongs in the OSS package rather than in
one service's config: a health signal that cannot distinguish "working" from
"not even trying" is worse than no signal, because it actively suppresses
investigation.
5 bootstrap tests pass; clippy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 05e2642 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
brentrager
added a commit
that referenced
this pull request
Aug 15, 2026
…thon, .NET (#87) * Stop reporting success when the SDK is exporting nothing — TS, Go, Python, .NET Ports the Rust fix from #84 to the other four SDKs. Every one of them had the same bug: `bootstrap()` reported `installed: true` whenever it was not explicitly disabled — INCLUDING when no OTLP endpoint was configured, in which case telemetry has nowhere to go. The no-endpoint branch was completely silent; it warned about missing AUTH but not about the far more consequential missing DESTINATION. TypeScript and Python are worse than Rust was. Rust at least skipped building the SDK with no endpoint. TS and Python construct an OTLP exporter with NO url, which the OTel default sends to `http://localhost:4318` — so a container with no endpoint configured doesn't no-op, it retries into the void forever. The Python test conftest already silences the resulting "connection refused to localhost:4318" spam, which is the bug leaving a note about itself. Per language: 1. An honest status flag alongside the existing one — `exporting` (TS, Python), `Exporting` (Go, .NET), matching Rust's `exporting`. `installed`/`Installed` keeps its old meaning and now says so honestly in its doc comment. Go derives it from the handle rather than the endpoint strings: an endpoint whose exporter failed to construct leaves every provider nil, and that is just as much "not exporting" as having no endpoint. .NET counts traces + metrics only — `Setup` builds exporters for exactly those two, and a LogsEndpoint alone is consumed by the ILoggingBuilder extension, so claiming Exporting on it would be the same lie in a new place. 2. A loud warning when no endpoint is configured, wording matched to Rust's: it names the variable to set AND offers SMOOAI_OBSERVABILITY_DISABLED=true so an intentional no-op can be declared rather than inferred from absence. 3. Both halves tested in all four — no endpoint ⇒ flag false, endpoint set ⇒ flag true. With only one asserted, an implementation that hard-codes either value passes; each assertion was mutation-checked to confirm it fails on its own. Three tests enshrined the misleading shape and now assert the honest one: Go's TestBootstrapInstallsClientAndCapture, Python's test_never_raises_on_bad_config, and .NET's Run_NeverThrows_OnBadConfig all asserted `installed` while nothing had a destination. Each also pins the warning now, and each clears the OTEL_EXPORTER_OTLP_* env vars so "no endpoint" means no endpoint from any source rather than whatever the CI runner happens to export. Also renames the .NET xUnit collection to OtelGlobalStateCollection — see the next commit for why it had to grow members. Gates, by exit code: TS typecheck/lint/test/build/format:check 0 (263 tests); Go gofmt/vet/test 0 across all three modules; ruff check + format --check 0, pytest 0 (78 tests); dotnet build/test/format 0 (80 tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * dotnet: unbreak the two CI gates this PR's lane would have failed on Both are pre-existing on origin/main and unrelated to the bootstrap change — but the dotnet lane only runs when dotnet/** changes, so this is the PR that has to face them. 1. `dotnet format --verify-no-changes` exits 2 on CrashChild.cs: 14 WHITESPACE errors, a braced switch-case body indented one level short. Verified identical on a stashed clean tree. Fixed by running `dotnet format` on that one file — pure indentation, no behavior. 2. OtelSetupTests.Setup_IsIdempotent is a flake, and a nasty one: it failed 3 of 8 full-suite runs on a clean tree (it passes 6 of 6 when the suite is filtered down, which is why it hid). ObservabilitySdk._installed is a process-wide static and three classes call ResetForTests() on it, but only BootstrapTests was in a collection. xUnit parallelizes ACROSS collections, so the other two ran concurrently with it and a foreign reset landed between that test's two Setup() calls, wiping the install guard the test exists to assert. Fixed by putting all three classes in one non-parallel collection (OtelGlobalStateCollection, renamed from "Bootstrap" since it guards the OTel singleton, not bootstrap). 10 of 10 full-suite runs green afterwards, verified by exit code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
This SDK told a production service it was fine while it emitted nothing for months.
bootstrap()returnedinstalled: truewhenever it wasn't explicitly disabled — including the case where no endpoint was configured, so no OTel SDK was installed and every span, metric and log went nowhere. The no-endpoint branch was completely silent: it warned about missing auth, but not about the far more consequential missing destination.A test enshrined it:
How it played out
smooai
chat-ws— the service running every LLM agent — set onlyOTEL_EXPORTER_OTLP_ENDPOINT, so this SDK installed nothing, while the operator's own gRPC exporter pointed at an endpoint that speaks authenticated HTTP/JSON. Production has 76gen_airows, all from one other service, none from chat-ws, ever. Nobody noticed, because everything reported healthy.Three changes
BootstrapResult::exporting— whether an OTLP exporter was actually installed.installedkeeps its old meaning (bootstrap ran) and now says so honestly in its doc comment instead of implying more.The no-endpoint branch warns, names the variable to set, and offers
SMOOAI_OBSERVABILITY_DISABLED=trueas the way to make the silence deliberate. An intentional no-op should be declared, not inferred from absence.Both halves tested — the old test now asserts
!exporting; a new one assertsexporting == truewhen an endpoint IS set. With only one, hard-coding either value passes.Why this belongs in the package, not in one service's config
A health signal that cannot distinguish "working" from "not even trying" is worse than no signal — it actively suppresses investigation. Every consumer of this crate inherits that trap; fixing it here fixes it for all of them.
5 bootstrap tests pass; clippy clean. Polyglot parity for the other four SDKs is in flight.