[Single Span Sampling] Add single span sampling rule#2091
Merged
Conversation
marcotc
force-pushed
the
single-span-rule
branch
from
June 17, 2022 16:31
b605277 to
61c7e02
Compare
Codecov Report
@@ Coverage Diff @@
## master #2091 +/- ##
==========================================
- Coverage 97.49% 97.49% -0.01%
==========================================
Files 1031 1034 +3
Lines 53216 53288 +72
==========================================
+ Hits 51882 51952 +70
- Misses 1334 1336 +2
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
* Add host information utility * fixup! Add host information utility * Rename file to platform * fixup! Rename file to platform * Require etc in file
* CI: Increase build and test jobs to medium+ * Reduce to small for small jobs
I'd like to thank @marcotc for adding the issue templates in #2090! Here's a few more tweaks inspired from the [issue templates](https://github.com/DataDog/libdatadog/tree/main/.github/ISSUE_TEMPLATE) we have for `libdatadog`. For the bug report template: * Mention log messages and screenshots in "steps to reproduce" suggestions * Add a "How does ddtrace help you" section. This helps us to know more about the customer's use case, and also gives some space for customers to hopefully tell us what we're doing well For the feature request template: * Add a few sections to guide the feature request.
Inspired by #2090 where we added templates for issues, I've decided to add a pull request template as well. I took this from <https://github.com/DataDog/libdatadog/tree/main/.github/ISSUE_TEMPLATE> where it has been serving us well, and all feedback is welcome :)
This PR fixes the YARD documentation return type of `c.tracing.partial_flush.min_spans_threshold`.
Adds a link to https://github.com/DataDog/dd-trace-rb/blob/master/docs/DevelopmentGuide.md to the PR template, to help developers test and clean up their code. Also, expand the language around testing requirements for the PR
ivoanjo
approved these changes
Jun 28, 2022
ivoanjo
left a comment
ivoanjo
left a comment
Member
There was a problem hiding this comment.
Left a few minor notes, but overall LGTM 👍
Comment on lines
+32
to
+33
| @sampler = Sampling::RateSampler.new | ||
| @sampler.sample_rate = sample_rate |
Member
There was a problem hiding this comment.
I think this can be simplified as
Suggested change
| @sampler = Sampling::RateSampler.new | |
| @sampler.sample_rate = sample_rate | |
| @sampler = Sampling::RateSampler.new(sample_rate) |
...or am I missing something? 🤔
Member
Author
There was a problem hiding this comment.
I added a comment explaining: it's an annoyance with how RateSampler validates initializer parameters.
Changing it would be a breaking change, so I added a note for a change for v2.0.
The Agent drops IDs == 0.
Doc-YARD:Fix documented type of min_spans_threshold
GH: Expand on PR template with link to docs
Co-authored-by: Ivo Anjo <[email protected]>
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
8 participants
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.
Follow up to #2055
This PR adds a sampling rule that applies a sampling rate (between 0% and 100% sampling) to a single span operation.
This rate is only applied if the provided matcher, introduced in #2055, is satisfied.
Additionally, a rate limiter (spans/sec limit) is applied to spans that satisfy both the matcher and sampling rate.
If the span is kept after all these checks, a few metrics are set in the span to let the Datadog App downstream know how the sampling happened: this allows it to adjust rates reported in the App despite receiving only a subset of the spans. It also allows users to know how much rate limiting is affecting their observability.
Also a reminder that the concept of "Single Span Sampling" only applies to spans that are part of traces that were dropped at trace-level sampling. Spans that are part of traces that have been successfully sampled do not need Single Span Sampling to be ingested.