[v3.0.0-alpha.1] Disable exporting traces by default

[mbentley]

Description

I thought I would update my registries to the 3.0.0-alpha.1 for testing and I am seeing that it looks like the registry is trying to connect to an endpoint for tracing but I have not configured anything.

Reproduce

1. start a registry as a registry mirror
2. pull images through it and eventually you'll see messages like this:

2024/01/31 21:13:05 traces export: Post "https://localhost:4318/v1/traces": dial tcp 127.0.0.1:4318: connect: connection refused
2024/01/31 21:13:10 traces export: Post "https://localhost:4318/v1/traces": dial tcp 127.0.0.1:4318: connect: connection refused
2024/01/31 21:13:15 traces export: Post "https://localhost:4318/v1/traces": dial tcp 127.0.0.1:4318: connect: connection refused
2024/01/31 21:13:42 traces export: Post "https://localhost:4318/v1/traces": dial tcp 127.0.0.1:4318: connect: connection refused
2024/01/31 21:25:22 traces export: Post "https://localhost:4318/v1/traces": dial tcp 127.0.0.1:4318: connect: connection refused
2024/01/31 21:25:27 traces export: Post "https://localhost:4318/v1/traces": dial tcp 127.0.0.1:4318: connect: connection refused
2024/01/31 21:25:52 traces export: Post "https://localhost:4318/v1/traces": dial tcp 127.0.0.1:4318: connect: connection refused

Expected behavior

I would expect that tracing is not enabled by default and is only enabled if configuration is passed.

registry version

3.0.0-alpha.1

Additional Info

Not sure if this comes from #4188 but I don't see docs that were written for that PR on the configuration.

[Thumbiceq]

I experience same issue with new image 3.0.0-alpha.1 Anyway I was able to turn it off by setting one of default env variable for opentelemetry -> https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/

specifically this env variable: OTEL_TRACES_EXPORTER=none

[mbentley]

I appreciate you sharing the OpenTelemetry docs @Thumbiceq! I saw a mention on one of the PRs about suggesting that we not use the config yaml since apparently it's standard for OpenTelemetry implementations on apps to use the env vars but hadn't looked into what it would take to explicitly disable it.

[ialidzhikov]

I appreciate you sharing the OpenTelemetry docs @Thumbiceq! I saw a mention on one of the PRs about suggesting that we not use the config yaml since apparently it's standard for OpenTelemetry implementations on apps to use the env vars but hadn't looked into what it would take to explicitly disable it.

@mbentley , can you please link this communication?

---

I would also expect the tracing to be off by default and explicitly enabled on demand via config option. By config option I mean the well-known config of the project, not and env var such as OTEL_TRACES_EXPORTER=none.

The fact that logs are spammed with

2024/02/05 13:53:18 traces export: Post "https://localhost:4318/v1/traces": dial tcp [::1]:4318: connect: connection refused

makes me think that either the default tracing thingy is wrongly configured by default or it requires additional things to make it work properly (maybe running something that serves on that port).

---

@gotgelf @corhere @milosgajdos could you help getting this issue fixed?

[ialidzhikov]

@mbentley , can you please link this communication?

I guess, that's it: #4184 (comment).

[mbentley]

Yes, that's it.

[corhere]

From https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/

The goal of this specification is to unify the environment variable names between different OpenTelemetry implementations.

Implementations MAY choose to allow configuration via the environment variables in this specification, but are not required to. If they do, they SHOULD use the names listed in this document.

We are following the OpenTelemetry Environment Variable Specification to the letter. Per the spec, the distribution daemon's OpenTelemetry integration is configured using the standardized environment variables, and will by default attempt to export telemetry traces to an OTLP collector at localhost:4318. If no OTLP collector is listening, the trace data is discarded. Aside from the unexpected log messages, how are these defaults problematic? Would you be satisfied if the new behaviour was documented?

• I have opened OTEL errors are not logged through Logrus #4275 to address the issue of OpenTelemetry SDK errors not being logged in the same format as other application logs.

[mbentley]

Just as an example, I don't see similar behavior when using BuildKit but I am not sure if it is a scenario where it is just not being logged. I would just expect that the default configuration would default to not being configured, expecting that something like tracing should be something where it explicitly needs to be enabled. While maybe not likely, I suppose a scenario could occur where there is a listening port host/port of 127.0.0.1:4318 so it would send data without the user enabling it and being aware. While not very likely and probabaly not very impactful, I think there is something to be said for predictable behavior when the default for many if not most people is to not need to configure OpenTelemetry.

[corhere]

Don't use BuildKit as an example of any OTEL best practices. It is abusing the OpenTelemetry SDK in awful ways which inflicts tremendous pain on the Moby project.

[corhere]

I would just expect that the default configuration would default to not being configured, expecting that something like tracing should be something where it explicitly needs to be enabled.

Someone familiar with the OpenTelemetry ecosystem might expect exporting spans to localhost:4318 using OTLP to be the default. That being said, I concede that while defaulting to tracing enabled is a reasonable default for an SDK which may be incorporated into bespoke software that is deployed only by its author, it might not be the right default for software projects such as distribution that are deployed by third parties. Neither nginx nor Envoy enable tracing by default, for example.

[gotgelf]

I would just expect that the default configuration would default to not being configured, expecting that something like tracing should be something where it explicitly needs to be enabled.

Someone familiar with the OpenTelemetry ecosystem might expect exporting spans to localhost:4318 using OTLP to be the default. That being said, I concede that while defaulting to tracing enabled is a reasonable default for an SDK which may be incorporated into bespoke software that is deployed only by its author, it might not be the right default for software projects such as distribution that are deployed by third parties. Neither nginx nor Envoy enable tracing by default, for example.

Thank you for bringing up this important consideration regarding the default configuration for tracing. It's clear that having sensible defaults that align with the expectations of both developers and third-party deployers is crucial for the usability and adoption of our software. Taking this into account, i see next potential paths to address this concern:

1. By Using OTEL_SDK_DISABLED (opentelemetry documentation) environment variable and setting it to true by default. During the OpenTelemetry initialization phase in the InitOpenTelemetry function within tracing.go, we would check this environment variable, and if it's set to disable the SDK, we would immediately return nil, effectively disabling the auto-exporter provided by the OpenTelemetry auto export library.
2. By Using OTEL_SDK_DISABLED set to trye by default, we can ensure that the OpenTelemetry functionality is completely turned off unless explicitly enabled (in this case we will need to adjust the places where we're using tracer).
3. Disabling Trace Exporting by Default: Setting the OTEL_TRACES_EXPORTER environment variable to none by default would specifically stop the exporting of traces. The auto-export library would recognize this configuration and not export any traces.

Wdyt?

[corhere]

Environment variables are for users to configure, and are literally global variables. We shouldn't tamper with them ourselves unless we have no other choice.

I don't think we should modify the default value of OTEL_SDK_DISABLED. It's a big hammer which also disables non-tracing aspects of the OTEL SDK, such as metrics. We aren't using OTEL for metrics export yet, but if we ever start, users who set OTEL_SDK_DISABLED=0 to enable tracing would be surprised to also have metrics implicitly enabled. And I highly doubt that many environments which have OpenTelemetry environment variables set would bother setting OTEL_SDK_DISABLED=0 because that's the default.

I think that the right way to disable exporting traces by default is to disable exporting traces by default. Frustratingly, the autoexport package almost makes it possible to do that without tampering with environment variables by providing WithFallbackSpanExporter but not exporting the noopSpanExporter type to pass into the option. While it is a trivial implementation to copy, the copy would be a distinct type and therefore autoexport.IsNoneSpanExporter(noopExporterCopy{}) would incorrectly return false.

@gotgelf would you have any interest in opening a feature request or PR on opentelemetry-go-contrib to afford changing the autoexport default exporters to no-ops without having to tamper with environment variables?

[gotgelf]

@gotgelf would you have any interest in opening a feature request or PR on opentelemetry-go-contrib to afford changing the autoexport default exporters to no-ops without having to tamper with environment variables?

Yes, definitely.

[gotgelf]

Hi @corhere ,

I've opened a related issue in the opentelemetry-go-contrib repository to address the default trace exporting behavior discussed here. You can find the issue here.