Setting any value to OTEL_EXPORTER_OTLP_PROTOCOL has no effect.

[moriyoshi]

Describe your environment

$ uname -srvmpio
Linux 4.19.128-microsoft-standard #1 SMP Tue Jun 23 12:58:10 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
$ python -VV
Python 3.9.9 (main, Dec 19 2021, 23:35:26)
[GCC 9.3.0]
$ grep ^opentelemetry pyproject.toml
opentelemetry-sdk = "^1.10.0"
opentelemetry-exporter-otlp-proto-http = "^1.10.0"
opentelemetry-exporter-otlp-proto-grpc = "^1.10.0"
opentelemetry-sdk-extension-aws = "^2.0.1"
opentelemetry-distro = "^0.29b0"
opentelemetry-instrumentation-django = "^0.29b0"

Steps to reproduce
I am using opentelemetry-instrument to wire up the components.

What is the expected behavior?
According to the documentation, one may choose the transport protocol for the OTLP exporter through OTEL_EXPORTER_OTLP_PROTOCOL, but it doesn't seem to apply to OTel Python, as the variable is referenced from nowhere.

What is the actual behavior?
Instead I had to set OTEL_*_EXPORTER to either otlp_proto_grpc or otlp_proto_http to change the protocol.

Additional context
The current SDK's architecture for the implementation lookup is not great IMO. This needs to be addressed.

[ronyis]

Hey, I would like to take this PR.

Some follow-up questions:

1. Do the options otlp_proto_grpc and otlp_proto_http should be removed for OTEL_*_EXPORTER? As far as I understand, this is behavior in Java SDK autoconfigure. It also makes more sense, otherwise users could define conflicting config by also using OTEL_EXPORTER_OTLP_PROTOCOL.

2. Does building the logs exporter should be based on OTEL_EXPORTER_OTLP_PROTOCOL and OTEL_EXPORTER_OTLP_LOGS_PROTOCOL? As it's not defined in the specs.

Thanks!

[srikanthccv]

The env would have more sense when we had one package. We decided to split the single package to multiple packages based on protocol + encoding so we have separate packages for gRPC, HTTP. I do not think this can easily be addressed without any breaking change.

[ronyis]

The SDK already supports initializing the configured exporter from the relevant package (in this module).
So I think this issue can be solved without changing the exporters packages structure.

But I do think that the breaking change required here is to remove the support for otlp_proto_grpc and otlp_proto_http for OTEL_*_EXPORTER.

[ronyis]

After some discussions, adding an overview of my proposed changes (all are related to traces, metrics & logs):

1. Updating SDK configurator to support the env vars OTEL_EXPORTER_OTLP_*_PROTOCOL defined by the specs. These envs will take effect only when OTEL_*_EXPORTER is set to otlp.

2. In case OTEL_*_EXPORTER will be set to otlp_proto_grpc/otlp_proto_http and a conflicting value will be set to OTEL_EXPORTER_OTLP_*_PROTOCOL, the protocol will be set according to OTEL_*_EXPORTER and a warning will be printed.

3. Updating the default value for OTEL_*_EXPORTER to otlp in opentelemetry-instrumentation package and in the SDK (also according to specs), as an actual value and not just an alias. The exporter used in this case will still be gRPC, to avoid a breaking change. This way, OTEL_EXPORTER_OTLP_*_PROTOCOL could be configured without specifically setting OTEL_*_EXPORTER=otlp.

---

Future breaking changes I propose is to remove the options otlp_proto_grpc/otlp_proto_http, since they are not defined in the specs, and they introduce this redundant complexity. Also, I didn't find that these are used in other runtimes.

Note - it does not require changing package entry points (such that), to avoid this unnecessary breaking change.

[srikanthccv]

Sounds good to me. It looks like the env are already defined here https://github.com/open-telemetry/opentelemetry-python/blob/edb1391a3d29d844141b15e486cd1107e163ee0b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py. If this requires only changes in contrib repo, I will transfer this issue there.

[ronyis]

It requires changes in both repos, so I think it can remain here.

[srikanthccv]

Ah correct, the configuration part was also originally part of opentelemetry-distro package but later it was moved to SDK. Since it requires changes in both, You will have to update SHA in test.yml when you open PRs for tests to pass; more details here https://github.com/open-telemetry/opentelemetry-python/blob/main/CONTRIBUTING.md#pull-requests