-
Notifications
You must be signed in to change notification settings - Fork 9
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
[SVLS-5049] It is okay to have a stats payload without stats #567
[SVLS-5049] It is okay to have a stats payload without stats #567
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #567 +/- ##
==========================================
+ Coverage 70.23% 70.39% +0.16%
==========================================
Files 214 214
Lines 28802 28901 +99
==========================================
+ Hits 20230 20346 +116
+ Misses 8572 8555 -17
|
BenchmarksComparisonBenchmark execution time: 2024-08-05 13:55:28 Comparing candidate commit 069d58d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 42 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
BaselineOmitted due to size. |
Before this change the agent had the following errors logged:
With this change the agent shows the following:
|
cd271c8
to
fdb470e
Compare
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.
LGTM!
fdb470e
to
069d58d
Compare
What does this PR do?
Prevents seemingly unnecessary errors if a Serverless stats payload happens to not have any stats.
The payload definition in the go protobuf code seems to indicate that stats are allowed to be empty. And that code (
firstService
closure) is defensive about this possibility. Furthermore ourdatadog.trace_agent.receiver.stats_buckets
metric has a minimum value of 0 so this apparently does happen and is handled as such by our systems.Motivation
The serverless agent is generating a bunch of noise when it receives these kinds of "empty" stats payloads from the node tracer.
How to test the change?
Added a unit test to confirm that we are able to create stats payloads with these empty stats objects (and empty stats of stats... our naming is a bit hard to follow). Also checking that this does in fact work with an azure function and that we correctly don't have errors there.