Skip to content

fix: batch pip installs in bootstrap for faster installs#4530

Open
Manvi2402 wants to merge 8 commits into
open-telemetry:mainfrom
Manvi2402:fix/batch-pip-install-4484
Open

fix: batch pip installs in bootstrap for faster installs#4530
Manvi2402 wants to merge 8 commits into
open-telemetry:mainfrom
Manvi2402:fix/batch-pip-install-4484

Conversation

@Manvi2402
Copy link
Copy Markdown
Contributor

Description

Previously, _run_install in bootstrap.py called pip install once per
package in a loop, causing multiple subprocess spawns, pip loads, and
dependency resolution passes. For a typical project with ~8 instrumentors,
this took 60-120 seconds.

This change batches all packages into a single pip install call, reducing
it to 15-25 seconds (4-6x speedup).

Fixes #4484

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?

  • Ran pre-commit checks — all passed
  • Verified syntax with python -m py_compile

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

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

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

@Manvi2402 Manvi2402 requested a review from a team as a code owner May 4, 2026 12:54
Comment thread CHANGELOG.md Outdated
### Fixed

- `opentelemetry-instrumentation`: Batch all packages into a single `pip install` call in `bootstrap.py` to improve performance
([#4484](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/4484))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs to be updated to link to the PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the changelog to link to the PR instead of the issue. Thanks for the review.

@github-project-automation github-project-automation Bot moved this to Reviewed PRs that need fixes in Python PR digest May 5, 2026
"-m",
"pip",
"install",
"-U",
Copy link
Copy Markdown

@idan-rahamim-lendbuzz idan-rahamim-lendbuzz May 5, 2026

Choose a reason for hiding this comment

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

Thanks For your contribution !
Can you also add to the opentelemetry-bootstrap -a install command a flag support to disable the -U of pip install?
In a fresh environment (e.g., Docker build), there's nothing to upgrade, so the upgrade check is pure overhead

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. Added a --no-upgrade flag to disable the -U option for fresh environments like Docker builds.

@Manvi2402 Manvi2402 requested a review from herin049 May 6, 2026 05:23
Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Nice optimization — batching pip installs is a cool thing to see.

A few things to tidy up:

  1. Error message broken for batched packagesf'{msg} for package "{packages}"' will print the list repr (e.g. for package "['pkg1', 'pkg2']"). Should join them, e.g. f'{msg} for packages "{", ".join(packages)}"'.

  2. _syscall wrapper is fragileif packages: is falsy for [], which would fall through to func() with no args and crash. Currently protected by the if libs: guard in _run_install, but the wrapper shouldn't rely on callers to not pass empty lists.

  3. --no-upgrade flag — this is a separate feature from the batching change. Consider splitting it into its own PR or at minimum mentioning it in the CHANGELOG entry.

  4. CHANGELOG section — this is a performance improvement, not a bug fix. Should be under a different section.

@Manvi2402 Manvi2402 force-pushed the fix/batch-pip-install-4484 branch from df930c4 to c711a18 Compare May 12, 2026 09:16
@Manvi2402
Copy link
Copy Markdown
Contributor Author

Hi @MikeGoldsmith , all review comments addressed!

Fixed error message to use ', '.join(packages) instead of list repr
Added empty list guard in _syscall wrapper
Removed --no-upgrade flag
Moved CHANGELOG entry to ### Changed section
Updated tests to reflect batched pip install behavior"

@emdneto
Copy link
Copy Markdown
Member

emdneto commented May 12, 2026

Thanks for the PR!

Just a heads-up: we no longer update CHANGELOG.md directly. The changelog is now generated from changelog fragments using Towncrier.

Please add the appropriate changelog fragment for this change instead of editing CHANGELOG.md manually. You can find the instructions and expected format in CONTRIBUTING.md.

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

Labels

None yet

Projects

Status: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

opentelemetry-bootstrap -a install is unnecessarily slow — recommend piping to pip install in docs

5 participants