dd-trace-js icon indicating copy to clipboard operation
dd-trace-js copied to clipboard

Skip 1.10.0+ bidi/stream GRPC server tests

Open jbertran opened this issue 1 year ago • 4 comments

What does this PR do?

  • Skip bidi and stream tests grpc server tests in >=1.10.0.

Motivation

The NodeJS GRPC implementation @grpc/grpc-js introduced in 1.10.0 server interceptors and removed/changed the previous events. Since then, bidi and stream requests experience unexpected cancellations which break tests for these.

We skip these in >1.10.0 to avoid polluting test results, pending correct instrumentation for this version.

Plugin Checklist

  • [x] Unit tests.

Additional Notes

Security

Datadog employees:

  • [ ] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • [x] This PR doesn't touch any of that.

jbertran avatar Feb 23 '24 14:02 jbertran

Overall package size

Self size: 6.11 MB Deduped: 61.99 MB No deduping: 62.75 MB

Dependency sizes

name version self size total size
@datadog/native-iast-taint-tracking 1.7.0 16.71 MB 16.72 MB
@datadog/native-appsec 7.0.0 14.51 MB 14.52 MB
@datadog/pprof 5.0.0 9.59 MB 10.44 MB
protobufjs 7.2.5 2.77 MB 6.56 MB
@datadog/native-iast-rewriter 2.2.3 2.19 MB 2.28 MB
@opentelemetry/core 1.14.0 872.87 kB 1.47 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
@opentelemetry/api 1.4.1 780.32 kB 780.32 kB
import-in-the-middle 1.7.3 67.62 kB 731.01 kB
pprof-format 2.0.7 588.12 kB 588.12 kB
msgpack-lite 0.1.26 201.16 kB 281.59 kB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.5.4 93.4 kB 123.8 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ipaddr.js 2.1.0 60.23 kB 60.23 kB
ignore 5.2.4 51.22 kB 51.22 kB
int64-buffer 0.1.10 49.18 kB 49.18 kB
shell-quote 1.8.1 44.96 kB 44.96 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
tlhunter-sorted-set 0.1.0 24.94 kB 24.94 kB
limiter 1.1.5 23.17 kB 23.17 kB
dc-polyfill 0.1.4 23.1 kB 23.1 kB
retry 0.13.1 18.85 kB 18.85 kB
node-abort-controller 3.1.1 16.89 kB 16.89 kB
jest-docblock 29.7.0 8.99 kB 12.76 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
methods 1.1.2 5.29 kB 5.29 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

github-actions[bot] avatar Feb 23 '24 14:02 github-actions[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.97%. Comparing base (0dee799) to head (cd74d17). Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4095      +/-   ##
==========================================
- Coverage   88.65%   84.97%   -3.68%     
==========================================
  Files         245      247       +2     
  Lines       10285    10730     +445     
  Branches       33       33              
==========================================
  Hits         9118     9118              
- Misses       1167     1612     +445     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 23 '24 14:02 codecov[bot]

It's unclear from the PR description if it's just the tests that are broken or the functionality. In any case, since this is a supported module, we need to make sure that it's working properly for new versions, otherwise tracing will stop working for users when they upgrade the module.

@rochdev as far as I can tell functionality is broken. This PR just skips these specific broken tests (bidi and stream calls) in >=1.10.0 so we don't miss other errors, but we do need to make them work with server interceptors.

jbertran avatar Feb 26 '24 10:02 jbertran

Benchmarks

Benchmark execution time: 2024-02-26 10:15:04

Comparing candidate commit cd74d171eef369b5870ab625ea1cbced69bafe9c in PR branch jbertran/grpc-skip-1.10.0+ with baseline commit 0dee799449b560b32c66077adc1cabafd34f2d44 in branch master.

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

pr-commenter[bot] avatar Feb 26 '24 10:02 pr-commenter[bot]

Closed by #4133

jbertran avatar Mar 08 '24 09:03 jbertran