Keep requests under the ingestion size limit - #25
Merged
Merged
Conversation
…are not logged A 401 is retried like a server error, and neither the status of a failed response nor a dropped batch shows up in NLog's internal log. The 408 and 429 cases pass already and pin that those stay retried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every non-success response now writes its status and reason phrase to NLog's internal log. Only responses worth another attempt are retried: no response, 408, 429 and 5xx. Any other status drops the batch right away with an error, which for 401 and 403 hints at the source token and the endpoint. A batch that runs out of retries is no longer dropped silently either. Based on the status code handling in #7. Co-authored-by: Rolf Kristensen <11509660+snakefoot@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
retries="0" sends nothing at all and retries="1" makes a single attempt. The two tests that ran a batch out of retries pinned that count and now expect one retry after the first attempt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The first attempt always happens, then up to `retries` retries with the same back-off. retries="0" sends every batch once instead of dropping it unsent, and the error line reports the real number of attempts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1000 logs with a 12 KB exception each make a single 12.8 MB request, over the 10 MB ingestion takes, and a 6 MB log is sent along with the rest of its batch. The small-batch test pins the exact request body, which must not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Client.Send serializes each log on its own and sends the batch in requests of at most 5 MB, in order, each with its own retries. Ingestion rejects a request over 10 MB with 413, which lost the whole batch. A single log too large for a request is dropped with an error naming its size. A batch that fits in one request is sent exactly as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 30, 2026
PetrHeinz
marked this pull request as ready for review
September 30, 2026 14:55
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…request-size-limit Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…imit Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
A batch goes out as a single request however large it is, because the drain caps batches by count only (1000 logs). Since 1.1.0 sends the exception of every log, a burst of errors fills that easily: 1000 errors with a 12 KB stack trace each, logged within one flush period, make a 12.9 MB request. Ingestion answered that with
413(it took 9.0 MB, the documented limit is 10 MB) and the whole batch was lost, exactly during an incident, when the backlog fills the batches.Client.Sendserializes each log on its own and sends the batch as one or more requests, each a JSON array of at most 5 MB (one constant, with a comment naming the 10 MB ingestion limit). The order of the logs is kept, and every request goes through the retry logic on its own.Send(IEnumerable<Log>)keeps its signature. It no longer posts an empty[]when it is given no logs (the drain never does that).The first commit adds the tests and is expected to be red: the 1000 logs arrive as one request of 12,764,828 bytes, and the 6 MB log is sent along with the logs around it. The small-batch test passes on both commits, it pins that the payload does not change. The second commit changes
Client.Send.The tests sit right before
SplitsLogsIntoBatchesinstead of after it, because #17 adds its test right after it.#25 and #26 both change
Client.serializeand conflict there. Whichever merges second serializes each log through #26's depth-limiting writer, and lets each log nest one level less than the limit (Top >= MaxDepth - 1), since the log then goes into the array of the request.CutsOffPropertyNestedTooDeeplycatches a missed adjustment: the test's JSON reader rejects a request nested deeper than 64 levels. A throwaway merge of all these branches with that resolution is green on NLog 4.7.11 and 6.*.🤖 Generated with Claude Code