Skip 1.10.0+ bidi/stream GRPC server tests #4095
Conversation
Overall package sizeSelf size: 6.11 MB Dependency sizes
🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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 ☔ View full report in Codecov by Sentry. |
| return server | ||
| }) | ||
|
|
||
| addHook({ name: '@grpc/grpc-js', versions: ['>=1.10.0'], file: 'build/src/server.js' }, server => { |
There was a problem hiding this comment.
We probably shouldn't be considering these versions supported at all until we have tests working to prove it actually works. 🤔
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
We probably shouldn't be considering these versions supported at all until we have tests working to prove it actually works. 🤔
That's fair. Since it makes package installs behave strangely for whatever reason, I've removed the new boundary anyway.
| return server | ||
| }) | ||
|
|
||
| addHook({ name: '@grpc/grpc-js', versions: ['>=1.10.0'], file: 'build/src/server.js' }, server => { |
There was a problem hiding this comment.
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.
jbertran
force-pushed
the
jbertran/grpc-skip-1.10.0+
branch
from
c384d7d to
cd74d17
Compare
@rochdev as far as I can tell functionality is broken. This PR just skips these specific broken tests (bidi and stream calls) in |
BenchmarksBenchmark execution time: 2024-02-26 10:15:04 Comparing candidate commit cd74d17 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 259 metrics, 7 unstable metrics. |
|
Closed by #4133 |
What does this PR do?
>=1.10.0.Motivation
The NodeJS GRPC implementation
@grpc/grpc-jsintroduced in1.10.0server interceptors and removed/changed the previous events. Since then,bidiandstreamrequests experience unexpected cancellations which break tests for these.We skip these in
>1.10.0to avoid polluting test results, pending correct instrumentation for this version.Plugin Checklist
Additional Notes
Security
Datadog employees:
@DataDog/security-design-and-guidance.