opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Remove unused `AsyncHooksContextManager`

Open dyladan opened this issue 1 year ago • 4 comments
trafficstars

          Also consider remove `AsyncHooksContextManager` because it will no longer be used.

Originally posted by @dyladan in https://github.com/open-telemetry/opentelemetry-js/issues/3720#issuecomment-1823194259

No longer used in SDK 2.0 so it can be safely removed.

dyladan avatar Dec 13 '23 16:12 dyladan

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Feb 26 '24 06:02 github-actions[bot]

@dyladan I'm currently refining issues, one question:

  • should we rename the package @opentelemetry/context-async-hooks to @opentelemetry/context-async-local-storage as it'll be the only context manager that's left in that package?

If yes, then I think we can break this down into multiple subtasks:

  • [ ] change all usages of AsyncHooksContextManager to AsyncLocalStorageContextManager
  • [ ] PENDING: rename package to @opentelemetry/context-async-local-storage
  • [ ] integrate abstract base AbstractAsyncHooksContextManager into AsyncLocalStorageContextManager

pichlermarc avatar Mar 08 '24 12:03 pichlermarc

Actually if we're going to do a rename we might be able to do this without breaking. I didn't even think of that. Do you think context-async-local-storage or context-node?

dyladan avatar May 30 '24 13:05 dyladan

Actually if we're going to do a rename we might be able to do this without breaking. I didn't even think of that. Do you think context-async-local-storage or context-node?

I think @opentelemetry/context-node may work.

So we'd do this:

  • [ ] create a new package @opentelemetry/context-node
  • [ ] copy AsyncLocalStorageContextManager to @opentelemetry/context-node and integrate the Abstract Base into that class
  • [ ] deprecate everything in @opentelemetry/context-async-hooks, refer to @opentelemetry/context-node and that it is the new one
  • [ ] deprecate @opentelemetry/context-async-hooks in NPM and remove the package from the repo

Then no tasks are left that are actually breaking and we can remove this issue from the milestone. WDYT @dyladan? If you agree then I'll create issues for the sub-tasks for contributors to pick up. Should be fairly easy tasks.

pichlermarc avatar Jun 25 '24 14:06 pichlermarc