Repository navigation
Conversation
A non-2xx response from the remote write endpoint was logged and then reported as success, so an invalid token or rejected batch looked healthy to the caller. Return an error carrying the status and the start of the response body for any non-2xx response.
The push interval was fixed at 15 seconds. Hosted receivers bill by samples per minute as well as active series, so a fixed 15 s interval costs four times what a 1 minute interval does for the same series. Default stays 15 s; non-positive values are rejected at startup. Also document --promRemoteURL in operations.md, and correct the statement that Loki telemetry is optional on mainnet.
jmgilman
requested review from
SEJeff,
bemic,
djb15,
evan-gray,
johnsaigle,
kcsongor,
mdulin2,
panoel and
pleasew8t
as code owners
October 7, 2026 23:17
SafeRead caps reads at MaxSafeInputSize (128MB). The remote write error path wrapped it in an io.LimitReader to keep the logged body small. SafeReadN takes the limit as an argument (1..MaxSafeInputSize) and SafeRead now delegates to it, so existing callers are unchanged. A rejected remote write with a body over 512 bytes now reports the status without the body instead of the first 512 bytes.
jmgilman
force-pushed
the
jmgilman/telemetry-metrics-warning
branch
from
October 8, 2026 16:19
5b8e99d to
b90cea5
Compare
Each push gathers and sends the whole metrics registry, so a very short interval loads both the node and the receiver. The flag exists to push less often; make the 15s default also the minimum.
Logs are already required on mainnet and testnet unless --disableTelemetry is set; metrics are not. Warn at startup outside devnet when --promRemoteURL is missing, and when --disableTelemetry is set while metrics are still pushed, ahead of a release that requires metrics and makes --disableTelemetry cover both. Behavior is otherwise unchanged.
jmgilman
force-pushed
the
jmgilman/telemetry-metrics-warning
branch
from
October 13, 2026 15:04
b90cea5 to
bb85c61
Compare
This branch has not been deployed
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.
Stacked on #5036: the first two commits are that PR, so review only the last one (
node: warn when Prometheus remote write is not configured). I'll rebase ontomainonce #5036 merges.On mainnet and testnet,
guardiandrefuses to start without--telemetryLokiURLunless--disableTelemetryis set. Metrics have no such default:--promRemoteURLis optional, and--disableTelemetrydoesn't affect it. We want Guardians to send both, with one switch to opt out of both.This PR announces the change without changing behavior. Outside devnet, at startup:
--promRemoteURLand no--disableTelemetry→ warn that a future release will require--promRemoteURL;--disableTelemetrywhile--promRemoteURLis set → warn that metrics are still pushed, and that a future release will stop pushing them when--disableTelemetryis set.operations.mddescribes both the current and the planned behavior.A follow-up release will turn the first warning into the same startup error Loki has and make
--disableTelemetrydisable metrics too. It will land once operators have their metrics URLs.