DEVPROD-11650 Don't recompute redactions for every Send invocation during test ingestion #8796
+42
−15
Conversation
This file contains 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
…ring test ingestion
Kimchelly
approved these changes
Mar 7, 2025
| // full list of redacted key-value pairs on initialization, or on every | ||
| // sender.Send invocation. This should not be set if the redacting sender | ||
| // is logging during task runtime, because expansions can change over the course | ||
| // of task runtime. It can only be safely set if we're confident that expansions |
There was a problem hiding this comment.
nit: remove double space if we're
| // sender.Send invocation. This should not be set if the redacting sender | ||
| // is logging during task runtime, because expansions can change over the course | ||
| // of task runtime. It can only be safely set if we're confident that expansions | ||
| // will no longer be changing. This flag exists to improve the performance of test log ingestion. |
There was a problem hiding this comment.
Might be more appropriate to move this last comment about the flag's use case (specifically about test log ingestion) over the line where it's set and just explain here that it's a performance optimization.
julianedwards
approved these changes
Mar 10, 2025
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.
DEVPROD-11650
Description
The redacting sender sorts the whole list of KV pairs to redact for every
.Send, which invokes for every log line. For tasks with many keys to sort (server can have 40+), this sorting operation can be computationally intensive and take up significant time.Since test log ingestion happens after the task has completed and the expansions will no longer be changing, we can instead precompute the sorted list of redacted KV pairs once on initializing the sender, rather than every time we process a line.
Testing
Ran benchmarking tests on very large log files and confirmed that this change improved the benchmark test runtime performance on local machine (5m -> 1m30s).