Utilize instruments-any in fastapi, kafka, and psycopg2

[jeremydvoss]

Description

Relies on #3610 which new full feature to support "either/or" instrumentation dependency scenarios in manual and autoinstrumentation. Utilizes this feature in fastapi, psycopg2, and kafka.

Fixes #3434

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

• Test A

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

[rjduffner]

So the python side is how we do the in code dependency checks.

The _instruments* are used by this function here

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-kafka-python/src/opentelemetry/instrumentation/kafka/__init__.py#L108-L126

Which is then loaded here. https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py#L88

During instrumentation time.

This change doesn't make sense to me as the logic for "either" is already inside the instrumentation_dependencies function. (https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-kafka-python/src/opentelemetry/instrumentation/kafka/__init__.py#L108-L126)

[jeremydvoss]

That get_dependencies method can be overwritten to achieve your desired result, though it's not its intended purpose. The problem is really that overriding it does not solve the scenario where we want to do dependency check before we load the instrumentation. That scenario is fundamental to cloud Auto instrumentation and it was the approach autoinsteumentation used prior to your change. This PR solves the problem from the ground up. Instrumentations no longer need to overwrite methods to provide hacky solutions for narrow scenarios. Now the SDK itself supports either/or scenarios -- whether the check happens before or after loading the instrumentation. In other words, with this change, partial solutions such as those implemented by Kafka and psycopg2 aren't required anymore. However they can still exist. That's why I haven't changed those method overwrites in the pr. This way it shouldn't break existing apps that already adopted that pattern. Let me know if that clears it up. We can sync tomorrow too or discuss in sig.

[rjduffner]

Sorry, I was mostly reacting to the comment which made it seem like this was still an open question. I do like that we may not need to use the instrumentation_dependencies method anymore but I am worried that its still there at all. Is it possible we could only do the toml method?

I do understand the need for handling more complicated scenarios, (a couple ifs and some ands etc), but in a case like this, the proposed solution in #3610 doesn't work either.

An exact example would be

• If package a, then package b and c are needed
• Or if package d then package e and f are needed

This logic gets super complicated super quickly which is why I went towards doing it in python via the instrumentation_dependencies method but I also totally get that we may not need any more complexity than and's and or's and we can maybe refactor more fully if when we ever need this.

[jeremydvoss]

Ah, i think I understand. Yeah this method works for all current cases, but I see what you mean about possible "branching" scenarios. This method does not have infinite customizability. I agree that a refactor like that would be a much bigger subject. My focus now is fixing all existing scenarios cleanly without the need for custom overwrites or hacks.

[jeremydvoss]

(Just a thought for what a long term solution that could support infinite branching might look like: a json-like "instruments" value that includes the logic within and the sdk merely executes.)

[jeremydvoss]

Missed that you were responding to my inline comment. I understand it but just forgot to remove my comment lol.

[rjduffner]

Open question why is this a different PR than #3610? Given #3610 changes this behavior shouldn't this all be rolled into one PR?