-
Notifications
You must be signed in to change notification settings - Fork 375
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
DEBUG-2334 Set duration to 0 for DI line probes #4134
Conversation
Line probes do not have a meaningful duration, however schema validation in system tests demands an integer value. Follow the behavior of other tracers and send 0.
02c4779
to
71c1ea4
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.
Default 0 make sense to me. Can remove other nil check from the caller.
I assumed to input duration can be nil
or Float
, otherwise, nil.to_i
can convert to 0 as well.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4134 +/- ##
==========================================
- Coverage 97.79% 97.78% -0.01%
==========================================
Files 1350 1350
Lines 81276 81322 +46
Branches 4107 4107
==========================================
+ Hits 79481 79519 +38
- Misses 1795 1803 +8 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
* psych-fix: Attempt to fix psych test flakiness DEBUG-2334 respect maxFieldCount in probe specification (#4142) Revert "Run the cucumber test in unbundled environment" (#4143) Update guard rails and doc (Excluding 3.4 installation) Lower declared requirements Use CentOS 7 images for SSI DEBUG-2334 require ostruct for OpenStruct dependency (#4135) DEBUG-2334 Set duration to 0 for DI line probes (#4134)
What does this PR do?
This PR changes the probe notification builder to set duration to 0 in the generated payload if it was not supplied by the caller.
Motivation:
Line probes do not have a meaningful duration, however schema validation in system tests demands a numeric value.
Follow the behavior of other tracers and send 0 instead of nil in this case.
Change log entry
None
Additional Notes:
This PR also includes additional assertions in the unit test for probe notification builder to make that test in general way more useful. Some of these assertions already exist in integration tests but here they are closer to the code.
How to test the change?
Tested by system tests (DataDog/system-tests#3516) and by the unit tests in this PR.