fix(tapper): stop retrying telemetry batches the hub rejects - #81
Merged
Conversation
A 400 was treated as retryable, so a client whose payload the hub will never accept re-sent the same rejected batch on every flush for the life of the process. The hub's decoder disallows unknown fields, so this is exactly what a client one version ahead of its hub does — it cannot negotiate the payload down, and no amount of retrying will help. Treat 400 like the other terminal statuses and disable the process reporter. Telemetry degrades to nothing, which is the correct outcome for a best-effort channel, and it lets the client and the hub release in either order rather than requiring the hub to go first. 413 stays retryable: batch contents vary, so a too-large batch says nothing about the next one.
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.
A 400 was treated as retryable, so a client whose payload the hub will never
accept re-sent the same rejected batch on every flush for the life of the
process.
The hub's telemetry decoder disallows unknown fields, so this is exactly what a
client one version ahead of its hub does: it cannot negotiate the payload down,
and no amount of retrying will help. The result is a silent stream of guaranteed
400s until the process exits.
Treat 400 like the other terminal statuses and disable the process reporter.
Telemetry degrades to nothing, which is the right outcome for a best-effort
channel, and it removes the ordering constraint between this repo and the hub —
either can release first.
413stays retryable on purpose: batch contents vary, so a too-large batch saysnothing about the next one.
Testing
New subtest asserts a 400 disables the reporter and the rejected batch is not
retried.
go build ./... && go test ./...green.