Adding Unidici support #4293
Adding Unidici support #4293
Conversation
Overall package sizeSelf size: 6.55 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 #4293 +/- ##
==========================================
- Coverage 87.35% 80.42% -6.93%
==========================================
Files 106 3 -103
Lines 3676 373 -3303
Branches 33 33
==========================================
- Hits 3211 300 -2911
+ Misses 465 73 -392 ☔ View full report in Codecov by Sentry. |
BenchmarksBenchmark execution time: 2024-05-24 23:54:05 Comparing candidate commit ac424fd in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 261 metrics, 4 unstable metrics. scenario:plugin-graphql-with-depth-on-max-18
|
tlhunter
force-pushed
the
undici_pr
branch
5 times, most recently
from
7387a6e
to
8dd1be7
Compare
| express = require('express') | ||
| fetch = require('undici') |
There was a problem hiding this comment.
Should probably change these to use withVersions(...) so we get version checking and don't need to add undici as a dev dependency.
You use it like this: https://github.com/DataDog/dd-trace-js/blob/master/packages/datadog-plugin-mysql/test/index.spec.js#L16
Then you can require or proxyquire it out of the versions folder. https://github.com/DataDog/dd-trace-js/blob/master/packages/datadog-plugin-mysql/test/index.spec.js#L31
It's a bit unintuitive, but that gets us multi-version checking.
If you want to see the withVersions(...) source to understand how it works, you can find it here: https://github.com/DataDog/dd-trace-js/blob/master/packages/dd-trace/test/setup/mocha.js#L169
crysmags
force-pushed
the
undici_pr
branch
2 times, most recently
from
f2e5643
to
b5778d1
Compare
|
|
||
| addHook({ | ||
| name: 'undici', | ||
| versions: ['^4.4.1', '5', '^6.0.0'] |
There was a problem hiding this comment.
The last range should be >=6 so that tracing will continue to work if a user upgrades to 7.0 when it's released.
| @@ -30,6 +30,7 @@ loadChannel.subscribe(({ name }) => { | |||
|
|
|||
| // Globals | |||
| maybeEnable(require('../../datadog-plugin-fetch/src')) | |||
| maybeEnable(require('../../datadog-plugin-undici/src')) | |||
There was a problem hiding this comment.
This shouldn't be needed since Undici is not global like fetch is.
| it('FIRST TEST FAILS BECAUSE.', () => { | ||
| }) |
There was a problem hiding this comment.
Leftover from test debugging? 🤔
There was a problem hiding this comment.
@Qard actually, it's not. There is an issue where whatever test that is first will always fail, hence this empty test. I am hoping that through review we could find a reason why/ or a more acceptable workaround.
There was a problem hiding this comment.
Strange. 🤔
What's the error?
What does this PR do?
This will add support for Undici for current versions of Undici that support fetch (versions '^4.4.1', '5', '^6.0.0'). It will also support diagnostics channel when it is supported by Undici.
Plugin Checklist
Additional Notes