search

open-telemetry / opentelemetry-python

OpenTelemetry Python API and SDK
https://opentelemetry.io
Apache License 2.0
1.76k stars 614 forks source link

Support for Jaeger Remote Sampling addition to contrib repo #3852

Closed sconover closed 3 months ago

sconover commented 5 months ago

Description

I'm attempting to port the jaeger remote sampling support from the jaeger-client-python project.

Per @srikanthccv 's feedback here, it really makes more sense for the bulk of the port to go into the contrib repo. However there are elements of the core sampler code that need to change to accommodate these contrib samplers.

I'm opening this in draft mode to seek initial feedback+guidance. There's a series of TODO's that raise questions about approach.

Type of change

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

Does This PR Require a Contrib Repo Change?

It doesn't require one, but there will be a companion PR that depends on this change.

Checklist:

srikanthccv commented 5 months ago

The sampler loading mechanism should be changed to entrypoint based. We already do this for several components. For example, we have exporters that users can configure with OTEL_{TRACES/LOGS/METRICS}_EXPORTER to otlp to load the OTELP exporters https://github.com/open-telemetry/opentelemetry-python/blob/642f8dd18eea2737b4f8cd2f6f4d08a7e569c4b2/exporter/opentelemetry-exporter-otlp/pyproject.toml#L32-L39

sconover commented 5 months ago

@srikanthccv Ok, upon a second look at this, it would appear that the library has the ability to load an arbitrary sampler from an env var, and that this validation is only used if the sampler selection falls back to default, explanation here. So I think there's nothing to do.