Skip to content
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

fix wrong test error message when expected span was not received #5447

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rochdev
Copy link
Member

@rochdev rochdev commented Mar 19, 2025

What does this PR do?

Fix wrong test error message when expected span was not received.

Motivation

Now that the official MsgPack library is used for decoding traces in the test agent, some values are BigInt instead of numbers which is not supported by JSON.stringify. This ends up throwing an error which gets outputted instead of the real underlying error that was thrown by the test.

rochdev added 2 commits March 19, 2025 18:24

Verified

This commit was signed with the committer’s verified signature.
adamscott Adam Scott
Copy link

Overall package size

Self size: 8.99 MB
Deduped: 101.52 MB
No deduping: 102.04 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.5.0 | 29.83 MB | 29.83 MB | | @datadog/native-appsec | 8.5.1 | 19.26 MB | 19.27 MB | | @datadog/native-iast-taint-tracking | 3.3.0 | 13.77 MB | 13.78 MB | | @datadog/pprof | 5.6.0 | 9.79 MB | 10.16 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.4.0 | 2.77 MB | 5.42 MB | | @datadog/native-iast-rewriter | 2.8.0 | 2.6 MB | 2.74 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.13.1 | 117.64 kB | 839.26 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | dc-polyfill | 0.1.6 | 24.56 kB | 24.56 kB | | shell-quote | 1.8.2 | 23.54 kB | 23.54 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.53%. Comparing base (38cb935) to head (fb0ac61).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5447      +/-   ##
==========================================
- Coverage   79.57%   79.53%   -0.04%     
==========================================
  Files         510      504       -6     
  Lines       22898    22763     -135     
==========================================
- Hits        18220    18104     -116     
+ Misses       4678     4659      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter
Copy link

pr-commenter bot commented Mar 19, 2025

Benchmarks

Benchmark execution time: 2025-03-19 23:25:21

Comparing candidate commit fb0ac61 in PR branch increase-kafka-span-tests-timeout with baseline commit 38cb935 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 916 metrics, 17 unstable metrics.

@datadog-datadog-prod-us1
Copy link

Datadog Report

Branch report: increase-kafka-span-tests-timeout
Commit report: c7bfb58
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 804 Passed, 0 Skipped, 15m 43.51s Total Time

@rochdev rochdev marked this pull request as ready for review March 19, 2025 23:57
@rochdev rochdev requested a review from a team as a code owner March 19, 2025 23:57
@@ -15,6 +15,7 @@ function resolveNaming (namingSchema) {

function expectSomeSpan (agent, expected, timeout) {
return agent.use(traces => {
const replacer = (k, v) => typeof v === 'bigint' ? Number(v) : v
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loss of precision, if some people debug values they're gonna be confused as to why the error is showing a weird value no ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it has to be JSON formatted, there is no way around that. We could return a string instead. But that might also be confusing?

@@ -33,7 +34,7 @@ function expectSomeSpan (agent, expected, timeout) {
const error = scoredErrors.sort((a, b) => a.score - b.score)[0].err
// We'll append all the spans to this error message so it's visible in test
// output.
error.message += '\n\nCandidate Traces:\n' + JSON.stringify(traces, null, 2)
error.message += '\n\nCandidate Traces:\n' + JSON.stringify(traces, replacer, 2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the error need to be JSON formatted? If not, lets use util.inspect()

Suggested change
error.message += '\n\nCandidate Traces:\n' + JSON.stringify(traces, replacer, 2)
error.message += '\n\nCandidate Traces:\n' + util.inspect(traces)

@@ -15,6 +15,7 @@ function resolveNaming (namingSchema) {

function expectSomeSpan (agent, expected, timeout) {
return agent.use(traces => {
const replacer = (k, v) => typeof v === 'bigint' ? Number(v) : v
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it has to be JSON formatted, there is no way around that. We could return a string instead. But that might also be confusing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants