-
Notifications
You must be signed in to change notification settings - Fork 195
Description
Bug description
thv config otel set-tracing-enabled false and thv config otel set-metrics-enabled false report success, but thv run ignores these values. Servers started without explicit --otel-tracing-enabled/--otel-metrics-enabled flags get tracingEnabled: true and metricsEnabled: true in their runconfig regardless of global config.
Two bugs contribute to this:
1. getTelemetryFromFlags skips TracingEnabled/MetricsEnabled fallback (functional)
In cmd/thv/app/run_flags.go, getTelemetryFromFlags applies global config fallbacks for 6 of 8 OTel fields but omits TracingEnabled and MetricsEnabled. Additionally, buildRunConfig passes raw runFlags.OtelTracingEnabled/runFlags.OtelMetricsEnabled to WithTelemetryConfigFromFlags, bypassing the fallback entirely for the proxy runner config.
Since both CLI flags default to true, global config is never consulted and servers always start with telemetry enabled.
2. omitempty on bool fields hides false in config.yaml (cosmetic)
In pkg/config/config.go, OpenTelemetryConfig uses omitempty on MetricsEnabled and TracingEnabled. Since false is Go's zero value, yaml.Marshal drops these fields from the YAML output. PR #3761 fixed this in pkg/telemetry/config.go (the runtime config struct) but not in pkg/config/config.go (the global application config struct).
Steps to reproduce
thv config otel set-tracing-enabled false
thv config otel get-tracing-enabled
# Output: Current OpenTelemetry tracing enabled: false
cat ~/Library/Application\ Support/toolhive/config.yaml | grep tracing
# (no output — omitempty drops the field)
thv run <any-server>
jq '.telemetry_config.tracingEnabled' \
~/Library/Application\ Support/toolhive/runconfigs/<server>.json
# Output: true (should be false)Expected behavior
When tracing-enabled or metrics-enabled is set to false in global config, thv run should respect that value unless overridden by an explicit CLI flag. The false value should also be visible in config.yaml.
Actual behavior
Global config values for tracing-enabled and metrics-enabled are silently ignored. Servers started without explicit CLI flags always start with both enabled (the CLI flag default), regardless of global config.
Additional context
- Related: Global OTel settings are not applied to servers started via API/UI #2295 (global OTel settings not applied via API/UI)
- Related: OTEL Configuration from Config File Not Being Read #1355 (prior fix in same function left these two fields out of the fallback)
- Related: OTel insecure setting isn't working in global config #1899 (same symptom for the
insecurefield, since fixed) - PR Fix tool counter per-request creation, omitempty config, and streamable-http version #3761 fixed
omitemptyinpkg/telemetry/config.gobut notpkg/config/config.go