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

Load instrumentors via Distro #480

Merged
merged 2 commits into from
May 4, 2021

Conversation

owais
Copy link
Contributor

@owais owais commented Apr 28, 2021

Description

This feature allows distros to customize instrumentation initialization when using automatically instrumenting with the opentelemetry-instrument command.

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

  • Tested manually
  • Added test for newly introduced method

Does This PR Require a Core Repo Change?

  • Yes.
  • No.

Checklist:

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

@owais owais requested a review from a team as a code owner April 28, 2021 22:36
@owais owais requested review from ocelotl and hectorhdzg and removed request for a team April 28, 2021 22:36
@owais
Copy link
Contributor Author

owais commented Apr 28, 2021

Ported PR open-telemetry/opentelemetry-python#1751 from core.

@owais
Copy link
Contributor Author

owais commented Apr 28, 2021

open-telemetry/opentelemetry-python#1751 (comment)

Actually, we have been using entry points to allow for environment variables to point to functions or classes (they can point to any Python object). So, any callback can be specified with an environment variable in this way.

@ocelotl answering your Q ^ from the core PR.

My specific use case is that I want to add additional functionality to some instrumentations with the help of hooks in the splunk-otel-python distribution. For example, I might want to override the logic to compute span names or want to add additional Splunk specific tags to spans. How do I do that today so that users running their apps with opentelemetry-instrument command but with splunk's distro installed automatically get Splunk specific behavior in instrumentations?

This feature will allow the Splunk distro to pass splunk specific kwargs to instrumentations when using the opentelemetry-instrument command. Sure, the distro can inject env vars for every single kwarg it wants to pass to every single instrumentation and we could update all instrumentations to support passing customization options via both kwargs and env vars including for python objects such as callbacks. It is technically doable but it is far more complicated and touches so many more packages, not to mention how ugly it'd make the distro code. This feature gives distros a clean and obvious way to override instrumentation initialization in a single place.

This commit makes the following changes:

- Introduces a new `load_instrumentor(EntryPoint) -> None:` with a
default implementation method to the `BaseDistro` class.
- The default implementation loads the insrumentor from the provided
entry point and calls applies it without any arguments. (same as before)
- sitecustomize now calls Distro's `load_instrumentor` method to load
and activate an instrumentor instead of doing it directly.
- Added a new `DefaultDistro` implementation which is used if not distro
is found by entry points.
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

👍

@codeboten codeboten merged commit 9199e3c into open-telemetry:main May 4, 2021
@owais owais deleted the instrumentation-loader branch January 26, 2022 08:23
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.

None yet

3 participants