Skip to content

cmd/utils: guard SampleRatio flag with IsSet check#34062

Merged
jwasinger merged 1 commit into
ethereum:masterfrom
vickkkkkyy:otel-sample-ratio-isset
Mar 23, 2026
Merged

cmd/utils: guard SampleRatio flag with IsSet check#34062
jwasinger merged 1 commit into
ethereum:masterfrom
vickkkkkyy:otel-sample-ratio-isset

Conversation

@vickkkkkyy
Copy link
Copy Markdown
Contributor

In setOpenTelemetry, all other fields (Enabled, Endpoint, AuthUser, AuthPassword, InstanceID, Tags) are guarded by ctx.IsSet() checks, so they only override the config file when explicitly set via CLI flags. SampleRatio was the only field missing this guard, causing the flag default (1.0) to always overwrite whatever was loaded from the config file.

  • Fix OpenTelemetry SampleRatio being unconditionally overwritten by the CLI flag default value (1.0), even when the user did not pass --rpc.telemetry.sample-ratio
  • This caused config file values for SampleRatio to be silently ignored

@jwasinger jwasinger merged commit 745b0a8 into ethereum:master Mar 23, 2026
6 of 8 checks passed
jrhea pushed a commit that referenced this pull request May 13, 2026
## Summary

The `--rpc.telemetry.sample-ratio` flag declares `Value: 1.0` and `geth
--help` advertises `(default: 1)`. In practice, however, omitting the
flag produces a sample ratio of `0`, causing
`sdktrace.TraceIDRatioBased(0)` to drop 100% of spans. Users who enable
`--rpc.telemetry` see the `OpenTelemetry trace export enabled` log line
and a clean startup, but no traces ever leave the process.

The root cause is the interaction between two pieces of code:

1. `cmd/utils/flags.go:setOpenTelemetry` (added in #34062) only copies
the flag value when `ctx.IsSet(...)` returns true:

   ```go
   if ctx.IsSet(RPCTelemetrySampleRatioFlag.Name) {
       tcfg.SampleRatio = ctx.Float64(RPCTelemetrySampleRatioFlag.Name)
   }
   ```

That is the right pattern for "don't clobber a config-file value with
the CLI default," but it implies that something else must initialise the
field when neither source sets it.

2. `node/defaults.go:DefaultConfig` never initialises
`OpenTelemetry.SampleRatio`, leaving it at the float64 zero value.

The result for the common CLI-only user (no TOML config) is `SampleRatio
= 0` → every span is silently dropped, despite the documented default of
1.

## Change

Seed `OpenTelemetry: OpenTelemetryConfig{SampleRatio: 1.0}` in
`node.DefaultConfig` so the documented default matches runtime behavior
and the `ctx.IsSet` guard in `setOpenTelemetry` continues to do what it
was designed to do.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants