Short-circuit StageData creation in trace generation#430
Merged
nikita-tkachenko-datadog merged 1 commit intoJun 3, 2024
Merged
Conversation
msbit01
force-pushed
the
429_short-circuit-stage-data-creation
branch
from
June 3, 2024 18:46
6d71158 to
cbf095a
Compare
nikita-tkachenko-datadog
approved these changes
Jun 3, 2024
nikita-tkachenko-datadog
left a comment
Collaborator
There was a problem hiding this comment.
Hi @msbit01, many thanks for submitting this fix!
It looks good enough to me, I agree that limiting the number of stages is a simple approach that is efficient enough, given that the second check that verifies the final JSON length is still there.
|
@nikita-tkachenko-datadog thanks for the rapid feedback! When do you think it would get released? |
Collaborator
I will try to do a release tomorrow |
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.
Requirements for Contributing to this repository
What does this PR do?
This is a fix for #429. This change will cause trace generation to short circuit before creating an unbounded number of StageData objects.
Description of the Change
Prevent creation of an unbounded number of StageData objects by limiting the number of objects created to be at most the number of characters allowed in the json object created from said objects. This is a conservative approach that could be refined but is simple and good enough in that it avoids unbounded object creation.
Alternate Designs
I considered computing the json and keeping track of the running length of the json object that would be created and checking to see if its length would exceed the limit. That would be a more precise approach than simply counting the number of stages but as my goal was just to bound the number of stages and not necessarily have the bound be perfect, I preferred this approach for its simplicity.
Possible Drawbacks
Verification Process
I confirmed that a test version of the plugin didn't result in the generation of millions of StageData objects as I had seen in previous tests.
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.