Khanayan123/implement span events #4386
Conversation
khanayan123
force-pushed
the
khanayan123/implement-span-events
branch
from
eb763c5
to
14b83e0
Compare
Overall package sizeSelf size: 6.65 MB Dependency sizes
🤖 This report was automatically generated by heaviest-objects-in-the-universe |
BenchmarksBenchmark execution time: 2024-06-10 19:27:15 Comparing candidate commit dffe200 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 261 metrics, 5 unstable metrics. |
khanayan123
force-pushed
the
khanayan123/implement-span-events
branch
from
0e41fac
to
a947a09
Compare
|
|
||
| const { _tags } = span._ddSpan.context() | ||
| expect(_tags).to.have.property(ERROR_TYPE, error.name) | ||
| expect(_tags).to.have.property(ERROR_TYPE, 'otel.recordException:' + error.name) |
There was a problem hiding this comment.
This workaround is kinda funky. If the temporary workarounds aren't great, I think it would be OK for now if the recordException API only creates the span event and doesn't update the error tracking tags
There was a problem hiding this comment.
Yeah it is a little funky, i updated the code to instead hack the tagger and the formatter rather then adding an identifier to the error_type property
khanayan123
force-pushed
the
khanayan123/implement-span-events
branch
from
dd02feb
to
81c43a3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4386 +/- ##
===========================================
+ Coverage 69.19% 93.18% +23.99%
===========================================
Files 1 98 +97
Lines 198 3067 +2869
Branches 33 33
===========================================
+ Hits 137 2858 +2721
- Misses 61 209 +148 ☔ View full report in Codecov by Sentry. |
| @@ -348,12 +348,24 @@ describe('OTel Span', () => { | |||
| } | |||
|
|
|||
| const error = new TestError() | |||
| span.recordException(error) | |||
| const datenow = Date.now() | |||
| span.recordException(error, datenow) | |||
There was a problem hiding this comment.
This should be tested to also work without the second parameter.
packages/dd-trace/src/format.js
Outdated
| const events = [] | ||
| if (span._events) { | ||
| for (const event of span._events) { | ||
| const formattedEvent = {} |
There was a problem hiding this comment.
Creating an object with no keys and then immediately mutating it like this is not recommended. Each modification creates a new hidden class with a new object shape whereas if you include all the keys, even if they are undefined initially, will produce a single shape and so will be faster. I would build the object in a single statement instead.
* implement span events and record exception api
* implement span events and record exception api
* implement span events and record exception api
* implement span events and record exception api
What does this PR do?
implements otel span events & recordException api as according to the RFC
Motivation
adding support for otel addEvent and recordException api
Notes
Tested against span events and recordException system tests on this branch
DataDog/system-tests#2539
There is a hack implemented in the tagger and formatter to ensure that the setting of error tags from the recordException api does not influence the setting of the trace.error bit