-
Notifications
You must be signed in to change notification settings - Fork 8
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
[APMSP-1013] Add stats exporter #584
[APMSP-1013] Add stats exporter #584
Conversation
53043a5
to
77af1cb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #584 +/- ##
==========================================
+ Coverage 73.19% 73.58% +0.39%
==========================================
Files 254 255 +1
Lines 36312 36941 +629
==========================================
+ Hits 26578 27183 +605
- Misses 9734 9758 +24
|
BenchmarksComparisonBenchmark execution time: 2024-09-20 08:13:26 Comparing candidate commit 3475380 in PR branch Found 1 performance improvements and 2 performance regressions! Performance is the same for 48 metrics, 2 unstable metrics. scenario:benching deserializing traces from msgpack to their internal representation
scenario:normalization/normalize_name/normalize_name/good
CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
BaselineOmitted due to size. |
You need to rebase on And I see your PR is into your own branch, so I'll keep quiet... |
ea7bee7
to
a517d6b
Compare
e9e4aa7
to
694a478
Compare
a2b6a86
to
e5a6b88
Compare
658871f
to
685eeb4
Compare
ae59bb7
to
d7c25af
Compare
9b5e087
to
8895da2
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.
Overall this looks good to me just a few small comments!
Is there anyway to run this with a real test application / real traces+stats and make sure they show up in the datadog UI? The unit tests look good but it's always nice to have a smoke test or equivalent
Co-authored-by: Andrew Glaude <andrew.glaude@datadoghq.com>
…e-computed-from-the-spans-payload
6a0e9ad
to
01cfcfb
Compare
.set_response_callback(Box::new(callback_wrapper)); | ||
if compute_stats { | ||
builder = builder.enable_stats(Duration::from_secs(10)) | ||
// TODO: Enable peer tags aggregation and stats by span_kind based on agent |
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.
non-blocking nit: I prefer putting the ticket ID right after the TODO.
e614416
to
b01b838
Compare
…e-computed-from-the-spans-payload
…e-computed-from-the-spans-payload
What does this PR do?
This PR add stats computation in the data pipeline crate by:
Motivation
The support for client side stats will allow using client side stats by default. This will reduce the number of span received by the agent.
Additional Notes
This PR does not allow fetching the list of peer tags from the /info endpoint of the agent.
How to test the change?
Describe here in detail how the change can be validated.