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

Add setuptools instrumentation install requirement #781

Conversation

mattoberle
Copy link
Contributor

@mattoberle mattoberle commented Oct 26, 2021

Description

The setuptools package is not part of the stdlib, but often available in the system environment (it is a buildtime requirement).

pkg_resources (a package provided by setuptools) is used as a runtime requirement in opentelemetry-instrumentation. Explicitly listing setuptools as a requirement protects instrumentation from breaking with import errors in cases where setuptools is not available system-wide.

For example:

  • A multi-stage Docker build where the final image does not contain buildtime requirements.
  • A build system that packages the runtime dependencies into a single binary.

This commit pins setuptools >= 16.0 because that is the first release that included all 5 imports instrumentation currently relies on.

Fixes #778

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

To verify the correct pin for setuptools I used git bisect, checking for the first version that included all imports used by opentelemetry-instrumentation. Further details can be found in the linked issue.

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated

@mattoberle mattoberle requested a review from a team October 26, 2021 16:22
@mattoberle mattoberle force-pushed the 778/instrumentation-setuptools-requirement branch from 3d55697 to c09090d Compare October 26, 2021 16:24
@mattoberle mattoberle changed the title Add setuptools intsrumentation install requirement Add setuptools instrumentation install requirement Oct 26, 2021
The `setuptools` package is not part of the stdlib, but often available
in the system environment (it is a buildtime requirement).

`pkg_resources` (a package provided by `setuptools`) is used as
a runtime requirement in `opentelemetry-instrumentation`. Explicitly
listing `setuptools` as a requirement protects instrumentation from
breaking with import errors in cases where `setuptools` is not available
system-wide.

For example:

* A multi-stage Docker build where the final image does not contain
buildtime requirements.
* A build system that packages the runtime dependencies into a single
binary.

This commit pins `setuptools >= 16.0` because that is the first release
that included all 5 imports instrumentation currently relies on.
@mattoberle mattoberle force-pushed the 778/instrumentation-setuptools-requirement branch from c09090d to 39ae6e2 Compare October 26, 2021 16:25
@owais owais requested a review from a team October 26, 2021 16:29
@owais
Copy link
Contributor

owais commented Oct 27, 2021

@open-telemetry/python-approvers please take a look

@owais owais merged commit 6912c39 into open-telemetry:main Oct 27, 2021
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.

Should "setuptools" be a runtime dependency of "opentelemetry-instrumentation"?
5 participants