node: default OpenTelemetry SampleRatio to 1.0#34948
Merged
Merged
Conversation
The --rpc.telemetry.sample-ratio CLI flag declares a default of 1.0, but cmd/utils.setOpenTelemetry only copies the flag value into the node config when ctx.IsSet returns true. Without a corresponding default on node.DefaultConfig.OpenTelemetry.SampleRatio, omitting the flag leaves the runtime ratio at the float64 zero value, which causes sdktrace.TraceIDRatioBased(0) to drop 100% of spans. Users see "OpenTelemetry trace export enabled" but no traces ever leave geth. Seed the default in DefaultConfig so the documented behavior matches runtime behavior.
2daeb58 to
e1f7bae
Compare
Contributor
|
Please remove the tests. Also please use |
jwasinger
approved these changes
May 12, 2026
Member
Author
|
Removed the tests |
fjl
approved these changes
May 12, 2026
jrhea
approved these changes
May 13, 2026
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
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.
Summary
The
--rpc.telemetry.sample-ratioflag declaresValue: 1.0andgeth --helpadvertises(default: 1). In practice, however, omitting the flag produces a sample ratio of0, causingsdktrace.TraceIDRatioBased(0)to drop 100% of spans. Users who enable--rpc.telemetrysee theOpenTelemetry trace export enabledlog line and a clean startup, but no traces ever leave the process.The root cause is the interaction between two pieces of code:
cmd/utils/flags.go:setOpenTelemetry(added in cmd/utils: guard SampleRatio flag with IsSet check #34062) only copies the flag value whenctx.IsSet(...)returns true: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.
node/defaults.go:DefaultConfignever initialisesOpenTelemetry.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}innode.DefaultConfigso the documented default matches runtime behavior and thectx.IsSetguard insetOpenTelemetrycontinues to do what it was designed to do.Test plan
--rpc.telemetry --rpc.telemetry.endpoint=grpc://collector:4317(no--rpc.telemetry.sample-ratioflag). Confirmed an OTLP gRPC connection is established and spans arrive at an OpenTelemetry collector.http://...:4318to confirm both transports work without the explicit flag.master(no flag → no spans), and confirmed the fix restores expected behavior.cmd/utilsfor the default-value path (not included; happy to add if reviewers prefer).