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

General environment configuration #1675

Closed
dyladan opened this issue Nov 11, 2020 · 4 comments · Fixed by #2111
Closed

General environment configuration #1675

dyladan opened this issue Nov 11, 2020 · 4 comments · Fixed by #2111
Assignees

Comments

@dyladan
Copy link
Member

dyladan commented Nov 11, 2020

Implement the general environment variable specification

Please use the getEnv('NAME') function exported by core instead of directly using process.env.NAME.

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-environment-variables.md#general-sdk-configuration

@dyladan dyladan changed the title Sampler environment configuration General environment configuration Nov 11, 2020
@jtmalinowski
Copy link
Contributor

Like I said in the original issue, I'm in progress on this.

@jtmalinowski
Copy link
Contributor

jtmalinowski commented Apr 10, 2021

Outstanding changes as of now:

  • OTEL_TRACES_SAMPLER and OTEL_TRACES_SAMPLER_ARG - I have a fairly complete implementation of this in a draft somewhere, just needs to be rebased
  • OTEL_SPAN* limits are incorrect versus spec, changing them at the rc stage is the best way to go, so new default limits kick in in 1.0 - very small change, I can do it
  • OTEL_TRACES_EXPORTER - kind of tricky, default is otlp for which JSON (aka web) support is experimental, so doesn't seem suitable for 1.0 (Exporter environment configuration #1676)
  • OTEL_METRICS_EXPORTER - as far as I remember, not planned for 1.0, is that correct?
  • OTEL_EXPORTER_ZIPKIN_ENDPOINT - very straightforward

Overall I think it's doable next week provided that we'll be able to review the code somewhat quickly. I'm saying, because I've been told there was a discussion about 1.0 during last SIG.

@jtmalinowski
Copy link
Contributor

@dyladan @vmarchaud should we discuss above in SIG this week or maybe you're able to decide right away?

@vmarchaud
Copy link
Member

vmarchaud commented Apr 10, 2021

I've linked #1676 for the OTEL_TRACES_EXPORTER since it was the original issue

OTEL_METRICS_EXPORTER - as far as I remember, not planned for 1.0, is that correct?

Yep, we only target tracing GA for now

OTEL_SPAN* limits are incorrect versus spec, changing them at the rc stage is the best way to go, so new default limits kick in in 1.0 - very small change, I can do it

As i see, its only the default values that are incorrect right ? If so thats indeed quite straitforward. I could do it tomorrow since i've planned some time to work on otel (if you are okay with that)

OTEL_TRACES_SAMPLER and OTEL_TRACES_SAMPLER_ARG - I have a fairly complete implementation of this in a draft somewhere, just needs to be rebased

Did you already opened a PR for this (can't find anything right now) ? Would be welcome if you do so we can start rewiewing it !

@dyladan @vmarchaud should we discuss above in SIG this week or maybe you're able to decide right away?

I think if we could open all PRs before the SIG that would be really great, maybe merge somes and make a first SDK RC if everything looks good

EDIT:

OTEL_EXPORTER_ZIPKIN_ENDPOINT - very straightforward

I'll do this too

@vmarchaud vmarchaud self-assigned this Apr 10, 2021
vmarchaud added a commit to vmarchaud/opentelemetry-js that referenced this issue Apr 11, 2021
vmarchaud added a commit to vmarchaud/opentelemetry-js that referenced this issue Apr 11, 2021
vmarchaud added a commit to vmarchaud/opentelemetry-js that referenced this issue Apr 11, 2021
vmarchaud added a commit to vmarchaud/opentelemetry-js that referenced this issue Apr 14, 2021
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants