-
Notifications
You must be signed in to change notification settings - Fork 16
[SVLS-7934] Log error details when trace request fails #1392
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
Conversation
BenchmarksComparisonBenchmark execution time: 2025-12-11 17:11:30 Comparing candidate commit a5f62e4 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 57 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
Group 13
Group 14
Group 15
Group 16
Group 17
Group 18
Group 19
BaselineOmitted due to size. |
| attempt = request_attempt, | ||
| max_retries = retry_strategy.max_retries(), | ||
| "Request failed with error" | ||
| "Request failed with error: {e:?}." |
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.
You can do this with an even smaller diff of just going from error = %e to error = ?e.
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.
Smaller and better! The change as is is making the event less structured. We should really keep the error in the error field.
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.
Done! Good to know this syntax!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1392 +/- ##
=======================================
Coverage 71.13% 71.14%
=======================================
Files 403 403
Lines 64140 64140
=======================================
+ Hits 45626 45630 +4
+ Misses 18514 18510 -4
🚀 New features to boost your workflow:
|
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Tests failed on this commit e76b915: What to do next?
|
63324d6 to
a5f62e4
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
Tests failed on this commit 983cf01: What to do next?
|
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What does this PR do?
When the request to send traces fails, log more details of the error.
Motivation
Right now the error message is too general, making it hard for customers and our engineers to debug network errors.
Example:
Additional Notes
How to test the change?
Deploy an AWS Lambda function in VPC. The Lambda uses Datadog extension, which uses this library.
Result
Before:
After: