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

Move _SUPPRESS_INTRUMENTATION key from instrumentation to api #2187

Merged
merged 3 commits into from
Oct 14, 2021

Conversation

owais
Copy link
Contributor

@owais owais commented Oct 9, 2021

Description

This will continue to be part of the internal API but live in the API
context package.

Fixes #2184

Type of change

Please delete options that are not relevant.

  • Internal enhancement/refactor
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Existing tests

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@owais owais requested a review from a team October 9, 2021 00:07
@owais owais force-pushed the remove-instrumentation-sdk-dep branch from 959a490 to d4a1f95 Compare October 9, 2021 00:14
@owais owais added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 9, 2021
@owais owais force-pushed the remove-instrumentation-sdk-dep branch from d4a1f95 to 54a3040 Compare October 9, 2021 00:15
@owais owais mentioned this pull request Oct 9, 2021
12 tasks
@owais owais force-pushed the remove-instrumentation-sdk-dep branch from 54a3040 to 494ee73 Compare October 9, 2021 00:25
@owais owais force-pushed the remove-instrumentation-sdk-dep branch 3 times, most recently from 582b458 to 3737ea5 Compare October 11, 2021 14:48
Copy link
Contributor

@lzchen lzchen left a comment

Choose a reason for hiding this comment

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

@owais
Are you going to be including the tox changes in this PR as well? (to remove opentelemetry-instrumentation dependency)

@owais
Copy link
Contributor Author

owais commented Oct 11, 2021

@lzchen it still has another dependency on opentelemetry-instrumentation which will be removed in #2188

@owais
Copy link
Contributor Author

owais commented Oct 11, 2021

@lzchen let's not merge this for now and wait for the release. I'd like all these related changes to go in together after the next release.

@ocelotl
Copy link
Contributor

ocelotl commented Oct 12, 2021

Marking it as draft to avoid accidental merging 👍

@ocelotl ocelotl marked this pull request as draft October 12, 2021 14:45
This will continue to be part of the internal API but live in the API
context package.
@owais owais force-pushed the remove-instrumentation-sdk-dep branch from 3737ea5 to 24444b4 Compare October 13, 2021 23:48
@owais owais marked this pull request as ready for review October 14, 2021 00:00
@owais owais enabled auto-merge (squash) October 14, 2021 00:01
@owais owais merged commit a746d57 into open-telemetry:main Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove SDK's dependency on opentelemetry-instrumentation
4 participants