opentelemetry-python-contrib icon indicating copy to clipboard operation
opentelemetry-python-contrib copied to clipboard

bootstrap: no need to install tortoiseorm instrumentation if pydantic is found

Open xrmx opened this issue 1 year ago • 4 comments

Description

Right now opentelemetry-bootstrap is installing tortoiseorm instrumentation if pydantic is found in the currently installed packages. In order to avoid that enhance bootstrap to recognize AND'ed requirements if instruments requirements are in a list.

Type of change

Please delete options that are not relevant.

  • [x] 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

  • [x] scripts/generate_instrumentation_bootstrap.py

Does This PR Require a Core Repo Change?

  • [ ] Yes. - Link to PR:
  • [x] No.

Checklist:

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

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

xrmx avatar Apr 11 '24 12:04 xrmx

cc @tonybaloney

xrmx avatar Apr 11 '24 12:04 xrmx

Hi @xrmx. What use cases would this update fix? e.g. startup errors, duplicate spans, errored spans

tammy-baylis-swi avatar Apr 17 '24 16:04 tammy-baylis-swi

Hi @xrmx. What use cases would this update fix? e.g. startup errors, duplicate spans, errored spans

bootstrap looks into this field for adding instrumentations based on the installed libraries. With that line tortoiseorm instrumentation is installed even in the case where just pydantic is installed.

xrmx avatar Apr 17 '24 19:04 xrmx

Revamped the PR to teach bootstrap about AND'ed dependencies. The idea here is to use lists instead of strings to express AND'ed dependencies (i.e. when the istrumentation requires more than one package to work) on top of the OR'ed dependencies supported right now. This is to fix tortoiseorm instrumentation that is installed by bootstrap even if the tortoiseorm library is not installed. ~This PR is failling because the semantic for pyproject.toml optional dependencies table that we are using at the moment does not permit to use lists but just strings. What do you think to stop using pyproject.toml optional dependencies table and use our own, e.g. [tool.opentelemetry-bootstrap]?~ In order to have this work I had to move the table used in pyproject from [project.optional-dependencies] to [tool.opentelemetry-bootstrap] which I think is a good change by itself since what we store there are not the instrumentantion package optional dependencies.

xrmx avatar May 27 '24 16:05 xrmx