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

Deprecate and reduce instrumenter usage internally #3151

Closed
sl0thentr0py opened this issue Jun 10, 2024 · 7 comments
Closed

Deprecate and reduce instrumenter usage internally #3151

sl0thentr0py opened this issue Jun 10, 2024 · 7 comments

Comments

@sl0thentr0py
Copy link
Member

Unclear currently how this is going to work in a backwards compat way

@szokeasaurusrex
Copy link
Member

@sl0thentr0py, I think it is okay for us to drop instrumenter without deprecating it first and without making the change backwards compatible, since we already document instrumenter as being "for internal use only."

Ideally, we would remove the instrumenter in the Phase 1 major release; however, I would argue that since instrumenter is explicitly mentioned as being for internal use, we can safely remove it even in a minor version.

@sl0thentr0py
Copy link
Member Author

We cannot remove the top level option though if someone is setting it in their init since that's breaking. We can remove all internal references but the init options has to be deprecated and stay till the major.
That being said, we will probably do a major anyway so let's see.

@szokeasaurusrex
Copy link
Member

Fair enough @sl0thentr0py. Although, I would imagine that few, if any, users set a custom instrumenter in their init, since we don't seem to document this functionality anywhere, as far as I can tell (e.g., instrumenter is missing from this page).

Worst case, I think we can get away with emitting a DeprecationWarning when instrumenter is set and otherwise ignoring the parameter's value (perhaps technically breaking, but it at least won't break people's apps).

@sl0thentr0py
Copy link
Member Author

it is required while using our span processor so far and is documented here
https://docs.sentry.io/platforms/python/tracing/instrumentation/opentelemetry/#usage

side note: while searching for docs usages, it is better as a dev to grep for a keyword in the docs repo rather than looking through the website to find something

@szokeasaurusrex
Copy link
Member

Okay @sl0thentr0py, thanks for the link and the tip on docs searching.

I think in that case, let's just keep the instrumenter option in the top-level API and emit a deprecation warning when the user passes the instrumenter parameter.

@szokeasaurusrex
Copy link
Member

szokeasaurusrex commented Jul 22, 2024

We have decided to split this task as follows:

@szokeasaurusrex
Copy link
Member

Closing in favor of #3320 and #3321.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants