-
Notifications
You must be signed in to change notification settings - Fork 580
test: stop using the obsolete AsyncHooksContextManager in tests #2864
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
test: stop using the obsolete AsyncHooksContextManager in tests #2864
Conversation
Since support for Node.js <14.8.0 was dropped, the modern AsyncLocalStorageContextManager can always be used. This was done for runtime code in open-telemetry/opentelemetry-js#4341 This PR does this for tests that were explicitly setting a context manager.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2864 +/- ##
==========================================
+ Coverage 89.65% 89.69% +0.04%
==========================================
Files 185 185
Lines 9034 9034
Branches 1852 1852
==========================================
+ Hits 8099 8103 +4
+ Misses 935 931 -4 🚀 New features to boost your workflow:
|
…pear on some spans where they did not before, presumably because AsyncLocalStorage is doing a better job tracing the async context
assertSingleSpan('cassandra-driver.execute', undefined, undefined, { | ||
[SEMATTRS_NET_PEER_NAME]: cassandraContactPoint, | ||
[SEMATTRS_NET_PEER_PORT]: 9042, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review note: the switch to AsyncLocalStorage for context management results in some spans getting net.peer.{name,port}
being applied where it was not before. Presumably this is because AsyncLocalStorage is doing a better job following the async-context and this code block now gets triggered.
This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature. |
Since support for Node.js <14.8.0 was dropped, the modern
AsyncLocalStorageContextManager can always be used. This was done
for runtime code in open-telemetry/opentelemetry-js#4341
This PR does this for tests that were explicitly setting a
context manager.