fix(observability): read T3CODE_OTEL_SDK_DISABLED before OTEL_SDK_DISABLED - #55
Conversation
An environment variable should not be able to countermand an endpoint the user configured in Settings or named with T3CODE_OTLP_*, which is the rule every other OTEL_* name already follows here. The kill switch a shared machine needs is now T3CODE_OTEL_SDK_DISABLED, a name T3 Code owns. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryMedium Risk Overview
Tests cover override, parsing, and end-to-end config/desktop resolution; ops and fork docs describe the new variable and precedence. Reviewed by Cursor Bugbot for commit 8c9ae36. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: TrogonStack/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: TrogonStack/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesTelemetry disablement controls
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant EnvironmentLoader
participant ServerConfig
participant DesktopOtlpExport
EnvironmentLoader->>ServerConfig: provide forceDisabled and signal settings
EnvironmentLoader->>DesktopOtlpExport: provide forceDisabled and signal settings
ServerConfig->>ServerConfig: clear all OTLP URLs when forceDisabled is true
DesktopOtlpExport->>DesktopOtlpExport: disable all signals when forceDisabled is true
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
Scoping OTEL_SDK_DISABLED to its own route was the wrong reading of the precedence rule. Precedence is about which name answers a setting, and turning export off is one setting: T3CODE_OTEL_SDK_DISABLED answers it, OTEL_SDK_DISABLED answers it when ours is unset. Asking ours first is what a machine needs to keep T3 Code exporting while its profile disables every other SDK. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
OTEL_SDK_DISABLEDcould answer it, so the answer came from whatever the machine happened to inherit.OTEL_SDK_DISABLED=truefor every other SDK on the box used to leave no way to get T3 Code's telemetry back except unsetting a variable the rest of the machine depends on.T3CODE_OTEL_SDK_DISABLED=falseis now that way.