-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework CI Visibility spans batching #379
Merged
nikita-tkachenko-datadog
merged 9 commits into
master
from
nikita-tkachenko/ci-visibility-batching
Jan 30, 2024
Merged
Rework CI Visibility spans batching #379
nikita-tkachenko-datadog
merged 9 commits into
master
from
nikita-tkachenko/ci-visibility-batching
Jan 30, 2024
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
nikita-tkachenko-datadog
force-pushed
the
nikita-tkachenko/ci-visibility-batching
branch
from
January 19, 2024 14:48
c042bdb
to
7edec11
Compare
nikita-tkachenko-datadog
added
the
changelog/Fixed
Fixed features results into a bug fix version bump
label
Jan 24, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't finish the review yet, but I've dropped some comments in the TracerWriter part.
src/main/java/org/datadog/jenkins/plugins/datadog/traces/write/TraceWriter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/traces/write/TraceWriter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/traces/write/TraceWriter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/traces/write/AgentTraceWriteStrategy.java
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/traces/write/AgentTraceWriteStrategy.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/traces/write/AgentTraceWriteStrategy.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/clients/DatadogAgentClient.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/clients/DatadogAgentClient.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/traces/DatadogTracePipelineLogic.java
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/clients/DatadogAgentClient.java
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/util/CircuitBreaker.java
Outdated
Show resolved
Hide resolved
src/main/java/org/datadog/jenkins/plugins/datadog/util/CircuitBreaker.java
Show resolved
Hide resolved
src/test/java/org/datadog/jenkins/plugins/datadog/clients/DatadogClientStub.java
Show resolved
Hide resolved
drodriguezhdez
approved these changes
Jan 29, 2024
nikita-tkachenko-datadog
deleted the
nikita-tkachenko/ci-visibility-batching
branch
January 30, 2024 14:21
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 PR adds logic that allows batching pipeline/job spans that the plugin submits: the logic that monitors jobs and pipelines calls the new
TraceWriter
to add a span to the current batch. The writer runs a background thread that gathers submitted spans into batches and submits them.Each batch includes up to 100 spans (configurable via env vars), and the thread polls the queue at least every 10 seconds (configurable via env vars) or more frequently if the queue has more spans than fit into a single batch.
The motivation for adding this is:
There was a batching mechanism that existed before the changes (
NonBlockingHttpClient
), however it did not address the two points above: different jobs could not be batched together, and all of the spans had to be submitted in one call (which mandated waiting until the end of the pipeline to accumulate them).Description of the Change
Alternate Designs
Possible Drawbacks
Verification Process
This does not change the plugin behaviour, so existing tests were used to verify there were no regressions.
In addition, manual tests were executed in a dockerized Jenkins instance, covering the following:
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/
label attached. If applicable it should have thebackward-incompatible
label attached.do-not-merge/
label attached.kind/
andseverity/
labels attached at least.