Skip to content

Conversation

@trentm
Copy link
Contributor

@trentm trentm commented May 28, 2025

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.

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
Copy link

codecov bot commented May 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.69%. Comparing base (6f65523) to head (f38ad20).
Report is 1 commits behind head on main.

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     

see 1 file with indirect coverage changes

🚀 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.

trentm added 2 commits May 28, 2025 14:07
…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,
});
Copy link
Contributor Author

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.

@github-actions
Copy link
Contributor

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.
Are you familiar with this package? Consider becoming a component owner.

@trentm trentm marked this pull request as ready for review May 29, 2025 16:17
@trentm trentm requested a review from a team as a code owner May 29, 2025 16:17
@trentm trentm merged commit 2ac08ff into open-telemetry:main May 30, 2025
23 checks passed
@trentm trentm deleted the trentm-bye-bye-AsyncHooksContextManager branch May 30, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment