Skip to content
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

chore: permanently delete opentelemetry-browser-extension-autoinjection #2661

Merged

Conversation

chancancode
Copy link
Contributor

@chancancode chancancode commented Jan 17, 2025

This package was archived in #2285, the code no logner builds and contains legacy/deprecated patterns that will stop working with SDK 2.0, which we have no intention to fix.

Added a link to the last snapshot of the code for those who are looking to reference the archived code.

Uses this GitHub-specific markdown annotation.


I found this while working on #2645, specifically this usage:

https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/archive/opentelemetry-browser-extension-autoinjection/src/instrumentation/WebInstrumentation.ts#L45-L81

Since the caller passes in an already-instantiated provider, this cannot be easily fixed. In any case, we clearly have no intention to release a new version of this package so it's a bit moot.

The intention was to freeze the code, but it's annoying that it comes up in the search for these type of migrations/refactor efforts. Looking at the commit history, it was already updated a handful of times by similar efforts despite the stated intention to keep the code frozen.

In #2285, @blumamir said:

I support removing the code (which one can also find in git by checking out to an early commit) and leaving just a note in the README to communicate the status so that it won't go away for some reasonable time.

The code pops up in IDE searches and may still promote old practices which I don't see any good reason to keep anymore.

...and @pichlermarc said...

Let's merge this PR and then we can still follow up on removing the code.

I agree with @blumamir so here I am finishing the job 😀

This package was archived in open-telemetry#2285, the code no logner builds and
contains legacy/deprecated patterns that will stop working with SDK
2.0, which we have no intention to fix.

Added a link to the last snapshot of the code for those who are
looking to reference the archived code.
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 🙂

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.96%. Comparing base (46accdc) to head (f03db69).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2661   +/-   ##
=======================================
  Coverage   90.96%   90.96%           
=======================================
  Files         172      172           
  Lines        8137     8137           
  Branches     1649     1649           
=======================================
  Hits         7402     7402           
  Misses        735      735           

@pichlermarc pichlermarc merged commit c504a9f into open-telemetry:main Jan 22, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants