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

Add none for exporter selection enums. #1439

Merged
merged 6 commits into from
Feb 15, 2021

Conversation

anuraaga
Copy link
Contributor

Changes

Adds none as an option for exporter selection. Because the default is otlp, there is no way to disable signal export via env vars.

@@ -141,11 +141,13 @@ Known values for OTEL_TRACES_EXPORTER are:
- `"otlp"`: [OTLP](./protocol/otlp.md)
- `"jaeger"`: [Jaeger gRPC](https://www.jaegertracing.io/docs/1.21/apis/#protobuf-via-grpc-stable)
- `"zipkin"`: [Zipkin](https://zipkin.io/zipkin-api/) (Defaults to [protobuf](https://github.com/openzipkin/zipkin-api/blob/master/zipkin.proto) format)
- `"none"`: Disables export of traces.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: No sure about "disable". If the application adds its own exporter, it probably won't get disabled by this setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks tried tweaking

CHANGELOG.md Outdated Show resolved Hide resolved
@carlosalberto carlosalberto added area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory spec:trace Related to the specification/trace directory labels Feb 12, 2021
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

I am confused about this change. What is the scenario that we want people to "disable" trace but still link all the trace SDK?

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

What is the scenario that we want people to "disable" trace but still link all the trace SDK?

We use this in the javaagent, so that vendors can disable the OTLP exporter and provide their own exporter, while still getting all of the other auto-configuration goodness.

@anuraaga
Copy link
Contributor Author

@bogdandrutu Yeah this is for the case we provide a batteries included package like the Java agent. Another case is wanting to only export traces without metrics.

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
@bogdandrutu
Copy link
Member

@anuraaga @trask thanks for the explanation. Next time please describe this in the PR description or an issue, it is important to make sure that we have good reasons (documented) for what we change/add :) Every API/property we add will add support cost so we need to understand why we are paying this cost :)

@carlosalberto carlosalberto merged commit e26d611 into open-telemetry:main Feb 15, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.