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

Supporting instrumented package version bumps in our instrumentation packages #550

Open
NathanielRN opened this issue Jun 24, 2021 · 3 comments

Comments

@NathanielRN
Copy link
Contributor

NathanielRN commented Jun 24, 2021

Description

Following the SIG meeting on June 24th, 2021, we recognized the need to come up with a plan for when instrumented packages bump their version in a way that breaks our existing instrumentation packages.

Auto instrumentation relies our packages indicating both the package and the version of the package they supported which is why we need PRs like #545.

In the SIG we outlined the following options with their PROs and CONs

Option 1: Release a new instrumentation package for every breaking new instrumented version bump

This is what Java does already.

E.g. if Flask version 2.0 doesn't break opentelemetry-instrumentation-flask-v1 we can keep using it? And then if Flask version 3.0 does then we only create opentelemetry-instrumentation-flask-v3.

PROs:

  • Tests/specialties for different versions separated into their own packages

CONs:

  • It is confusing to have multiple opentelemetry-instrumentation-<pkgs>-v* packages
  • We should probably deprecate the original opentelemetry-instrumentation-<pkgs>- package
  • Every time names/fixes/features are added we need to apply it to all these different packages instead of just 1 package at a time

Option 2: Use 1 instrumentation package for all versions of the instrumented package

PROs:

  • No confusion regarding which package we need to use

CONs:

  • Testing is more complicated as we need to download 2+ versions of the same package to test different tests?
  • Requires potentially very ugly logic branching to accommodate the different versions

Additional noteworthy points

@owais
Copy link
Contributor

owais commented Jul 1, 2021

Option 2 makes a lot of sense to me. Most importantly, it's very obvious for end users. I think in general we should not make things harder for users just to make maintenance a bit easier for ourselves unless of course it gets very hard to do.

Testing is more complicated as we need to download 2+ versions of the same package to test different tests?

I think this is a test matrix / tox issue and we should solve it there instead of splitting packages. We already do this for elasticsearch: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/tox.ini#L172-L179

Requires potentially very ugly logic branching to accommodate the different versions

This is a very valid point but I think we can make it work with some re-organization and split the code into re-usable functions/classes.

I think we can still create dedicated instrumentation packages for different versions of libraries if things get too bad but I think this should be an exception rather than a rule and should be evaluated on a per package basis.

@codeboten
Copy link
Contributor

Please vote with:
Option 1 🚢
Option 2 🚀

@lzchen
Copy link
Contributor

lzchen commented Jul 8, 2021

From SIG discussions: since major version updates are not common, the cons listed for option 2 should be manageable/minimal.

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

No branches or pull requests

4 participants