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

Migrate instrumentation and disro to contrib #2196

Merged
merged 1 commit into from Oct 14, 2021

Conversation

owais
Copy link
Contributor

@owais owais commented Oct 14, 2021

Description

Now that SDK does not depend on opentelemetry-instrumentation anymore and opentelemetry-instrumentation has actual build time dependencies on the contrib repo, it makes maintanence a lot easier if we move opentelemetry-instrumentation to contrib repo. opentelemetry-distro depends on opentelemetry-instrumentation and is being moved as well. Neither of the two packages are really part of "core" Otel python anyway.

Type of change

Please delete options that are not relevant.

  • 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?

Checklist:

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

@owais owais added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 14, 2021
@owais owais force-pushed the remove-instrumentation-package branch 3 times, most recently from 21b3aec to a649d5d Compare October 14, 2021 01:46
@owais owais changed the title Remove instrumentation package Migrate instrumentation and disro to contrib Oct 14, 2021
@owais owais force-pushed the remove-instrumentation-package branch 2 times, most recently from e1233a8 to ed25964 Compare October 14, 2021 02:31
@owais owais marked this pull request as ready for review October 14, 2021 02:35
@owais owais requested a review from a team as a code owner October 14, 2021 02:35
Copy link
Contributor

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

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

I am ok with this since it makes maintenance easier, nevertheless:

  1. I feel like maintenance should not be hard in the first place, looks like we could benefit from a way of installation of packages that is not made harder or easier depending on where the code of that package is.
  2. We need to write down the criteria that decides what goes into the core repo and what goes into contrib.
  3. I would expect at least opentelemetry-instrumentation to go back to the core repo sooner than later, instrumentation is at least mentionedhere.

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.

N I C E

Now that SDK does not depend on opentelemetry-instrumentation
anymore and opentelemetry-instrumentation has actual build time
dependencies on the contrib repo, it makes maintanence a lot
easier if we move opentelemetry-instrumentation to contrib repo.
opentelemetry-distro depends on opentelemetry-instrumentation
and is being moved as well. Neither of the two packages are
really part of "core" Otel python anyway.
@owais owais force-pushed the remove-instrumentation-package branch from ed25964 to 3adcdfc Compare October 14, 2021 20:17
@owais owais enabled auto-merge (squash) October 14, 2021 20:18
@owais owais disabled auto-merge October 14, 2021 20:44
@owais owais merged commit 3ef7cb5 into open-telemetry:main Oct 14, 2021
@owais owais deleted the remove-instrumentation-package branch October 14, 2021 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants