-
Notifications
You must be signed in to change notification settings - Fork 10
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
Initial move to use SendData in trace exporter (APMSP-1586) #753
Conversation
BenchmarksComparisonBenchmark execution time: 2024-11-22 21:27:24 Comparing candidate commit 8a883de in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 51 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
Group 12
BaselineOmitted due to size. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #753 +/- ##
==========================================
- Coverage 70.79% 70.50% -0.30%
==========================================
Files 297 297
Lines 43568 43385 -183
==========================================
- Hits 30843 30587 -256
- Misses 12725 12798 +73
|
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.
Rough, to use your words but LGTM since we're addressing all the issues imminently.
Please associate a Jira ticket to this PR before merging. |
Despite the compromises we need to make it LGTM. |
What does this PR do?
Moves TraceExporter to use trace utils send data functionality.
Motivation
We want access to the retry functionality of trace utils as well as to start supporting TinyBytes so we have good performance here
Additional Notes
There are a number of things to do after this is merged that have been commented out explicitly so that we can unblock others working in this area. (e.g. client stats)
How to test the change?
Covered with unit and integration tests for now