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 OTEL_EXPORTER environment variable #943

Merged
merged 4 commits into from
Sep 16, 2020

Conversation

trask
Copy link
Member

@trask trask commented Sep 14, 2020

Fixes #710

Changes

Adds OTEL_EXPORTER environment variable to select the exporter to be used.

Adds OTEL_EXPORTER_OTLP_ENDPOINT environment variable to configure the ingest endpoint for both OTLP spans and metrics.

Notes

  1. I added OTEL_EXPORTER_OTLP_ENDPOINT in this PR because I think it makes the default value OTEL_EXPORTER=otlp make more sense.

    (It also happens to be the env var that the Java SDK uses currently, and seems simpler for most OTLP users, who will be sending spans and metrics to the same endpoint)

    An alternative would be to specify the default value as OTEL_EXPORTER=otlp_span,otlp_metric.

  2. I went with the singular OTEL_EXPORTER over the plural OTEL_EXPORTERS, since the common use case is to configure just one exporter and so this seems less confusing.

@trask trask requested review from a team September 14, 2020 05:05
specification/sdk-environment-variables.md Outdated Show resolved Hide resolved
specification/sdk-environment-variables.md Outdated Show resolved Hide resolved
specification/sdk-environment-variables.md Outdated Show resolved Hide resolved
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.

Is there a mention in the spec as to what the known values that should be used to identify exporters are and how exporters should register these values? That would be useful for anyone implementing additional exporters.

specification/sdk-environment-variables.md Outdated Show resolved Hide resolved
specification/sdk-environment-variables.md Outdated Show resolved Hide resolved
@SergeyKanzhelev
Copy link
Member

Adds OTEL_EXPORTER environment variable to select the exporter to be used.

would it make more sense for every exporter to keep _ENABLED variable (like OTEL_EXPORTER_OTLP_SPAN_ENABELD and OTEL_EXPORTER_JAEGER_ENABLED)? This way we can enable any number of exporters and no need to keep the centralized list. Also it is more extensible as the list of exporters can be extended in very logical way.

Having OTEL_EXPORTER is more aligned with the existence of a single batch processor setting. But I'd argue it's more logical to have enable flag configured with the exporter.

@trask
Copy link
Member Author

trask commented Sep 15, 2020

There are two alternatives suggested above. I like both of them, except around handling of the default value.

  1. Suggestion: split into two env vars, one for span exporter (OTEL_EXPORTER_SPAN) and one for metric exporter (OTEL_EXPORTER_METRIC)

    The default in this case would be OTEL_EXPORTER_SPAN=otlp and OTEL_EXPORTER_METRIC=otlp.

    If someone wants to use jaeger, would they need to set both OTEL_EXPORTER_SPAN=jaeger and OTEL_EXPORTER_METRIC=none?

  2. Suggestion: split out into a boolean flag for each exporter (OTEL_EXPORTER_*_ENABLED)

    The default in this case would be OTEL_EXPORTER_OTLP_ENABLED=true and all others OTEL_EXPORTER_*_ENABLED=false.

    If someone wants to use jaeger, would they need to set both OTEL_EXPORTER_JAEGER_ENABLED=true and OTEL_EXPORTER_OTLP_ENABLED=false.

Wondering if anyone has suggestions how to handle default value better in either of these cases? Or should we stick with the single environment variable OTEL_EXPORTER with default value otlp?

@iNikem
Copy link
Contributor

iNikem commented Sep 15, 2020

I prefer the third alternative, which is currently written in PR :)

@trask
Copy link
Member Author

trask commented Sep 15, 2020

In case we go with OTEL_EXPORTER, and because I ❤️ naming, does anyone care between:

  • OTEL_EXPORTER=otlp_span
  • OTEL_EXPORTER=otlp-span
  • OTEL_EXPORTER=otlpspan

Exporter names seem sort of like propagator names, not sure if there's any convention we can follow from those?

@iNikem
Copy link
Contributor

iNikem commented Sep 15, 2020

For some reason OTEL_EXPORTER=otlp_span seems the most natural to me

@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Sep 15, 2020

@trask you got me on default values. Single variable is better from this perspective.

As for naming, I again tend to lean to separate variables for metrics and spans. It feels like two separate pieces of configuration semantically.

Since I also ❤️ naming, should we get PM to this PR to help nail the desired semantic of this config settings ;-)?

(edit: I assumed "because I ❤️ naming" was a sarcasm. If not, than mine is =))

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.

Thanks for the updates, this LGTM

@trask
Copy link
Member Author

trask commented Sep 15, 2020

(edit: I assumed "because I ❤️ naming" was a sarcasm. If not, than mine is =))

Mine was intentionally ambiguous 😂. I have a complicated relationship with naming.

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM

@SergeyKanzhelev
Copy link
Member

Last edit was a day ago, so plan to merge tomorrow morning unless somebody will comment with blocking concerns

@owais
Copy link
Contributor

owais commented Sep 15, 2020

Other than OTLP, most other exporters expect an explicit value for "service name". OTLP expects a service_name resource attribute. The spec already specifies a way to configure resource attributes globally for a service using environment variables (OTEL_RESOURCE_ATTRIBUTES) so OTLP is covered but this leaves other exporters without a way to set the service name. We can either add a common OTEL_SERVICE_NAME env var or one env var per exporter like OTEL_EXPORTER_JAEGER_SERVICE_NAME.

@trask
Copy link
Member Author

trask commented Sep 15, 2020

We can either add a common OTEL_SERVICE_NAME env var or one env var per exporter like OTEL_EXPORTER_JAEGER_SERVICE_NAME.

Hey @owais, check out (and vote for!) #709

@trask trask deleted the otel-exporter-env-var branch October 14, 2024 21:05
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
* Add OTEL_EXPORTER environment variable

* feedback

* Add note about otlp=otlp_span,otlp_metric
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.

Add OTEL_EXPORTER environment variable
9 participants