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 22:49
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.
djb15
previously approved these changes
Oct 8, 2026
pleasew8t
reviewed
Oct 13, 2026
pleasew8t
reviewed
Oct 13, 2026
| // SafeReadN is SafeRead with a caller-chosen limit of n bytes, for callers that need a tighter | ||
| // bound than MaxSafeInputSize. It returns ErrInputTooLarge if the input exceeds n bytes, and | ||
| // ErrInvalidReadLimit if n is not between 1 and MaxSafeInputSize. | ||
| func SafeReadN(r io.Reader, n int) ([]byte, error) { |
Contributor
There was a problem hiding this comment.
I'd recommend changing SafeReadN() to accept uint instead of int to avoid any casting requirements, since I don't think there's a reason for a caller to ever specify a negative value? The first if-statement could then be updated to if n == 0 || n > MaxSafeInputSize.
Contributor
Author
There was a problem hiding this comment.
I tried uint: it actually adds a cast (uint(len(b)) > n, since len returns int) rather than removing one, and int64(n) for io.LimitReader is needed either way. I kept int to match Go's convention for sizes (len, make, bufio.NewReaderSize). Negative values are also rejected with ErrInvalidReadLimit and covered by a test.
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.
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
3 participants
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.
Two changes to the Prometheus remote write push (
--promRemoteURL).Rejected pushes were reported as success. On a non-2xx response,
ScrapeAndSendLocalMetricslogged the status and returned the outererr, which is nil at that point. An operator with a wrong or expired token saw a node that looked healthy while every push was dropped. Any non-2xx now returnsremote write returned status N: <body>, with the receiver's reason (up to 512 bytes), e.g. Grafana'sauthentication error: invalid token.The push interval was fixed at 15 seconds. Hosted receivers such as Grafana Cloud bill by samples per minute as well as by active series, so 15 s costs about four times what 1 min does for the same metrics. The new
--promRemoteIntervalflag (alsoGUARDIAND_PROMREMOTEINTERVALor the config file) sets the interval. The default stays 15 s, so nothing changes unless an operator sets it. A zero or negative value stops the node at startup.Docs:
operations.mdnow documents--promRemoteURLand the interval. It also corrects the claim that Loki telemetry is optional: on mainnet and testnet the node won't start without--telemetryLokiURLunless--disableTelemetryis set.