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 common language-agnostic environment variable configuration docs #1432

Merged
merged 8 commits into from
Jun 10, 2022

Conversation

cartermp
Copy link
Contributor

@cartermp cartermp commented Jun 8, 2022

Preview: https://deploy-preview-1432--opentelemetry.netlify.app/docs/concepts/sdk-configuration/

Okay, so I used the word "common" here, but this is mostly based on what I've seen people need to configure to successfully send data to a backend. There's a lot more stuff in the spec that could be pulled in here, but since perfect is the enemy of good, I figured I'd get some docs going that aren't just links to the spec.

@cartermp cartermp marked this pull request as ready for review June 8, 2022 19:55
@cartermp cartermp requested a review from a team June 8, 2022 19:55
Copy link
Member

@austinlparker austinlparker left a comment

Choose a reason for hiding this comment

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

Several problems in the example env vars.

cartermp and others added 2 commits June 8, 2022 13:22
Co-authored-by: Austin Parker <austin@ap2.io>
cartermp and others added 3 commits June 9, 2022 09:27
@austinlparker
Copy link
Member

this lgtm -- g2g @cartermp?

@cartermp
Copy link
Contributor Author

Yep, I can always add more in future PRs

@austinlparker austinlparker merged commit 3c3feb3 into open-telemetry:main Jun 10, 2022
@cartermp cartermp deleted the cartermp.sdk-config-docs branch June 10, 2022 18:14
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

I know it is merged but I have a few more comments


## Endpoint Configuration

The following environment variables let you configure an OTLP/gRPC or OTLP/HTTP
Copy link
Member

Choose a reason for hiding this comment

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

Following env vars are not documented:

  • OTEL_EXPORTER_OTLP_PROTOCOL
  • OTEL_EXPORTER_OTLP_TRACES_PROTOCOL
  • OTEL_EXPORTER_OTLP_METRICS_PROTOCOL


**Example:**

* gRPC: `export OTEL_EXPORTER_OTLP_ENDPOINT="my-api-endpoint:443"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* gRPC: `export OTEL_EXPORTER_OTLP_ENDPOINT="my-api-endpoint:443"`
* gRPC: `export OTEL_EXPORTER_OTLP_ENDPOINT="https://my-api-endpoint"`

AFAIK all (or most) of the languages require http or https prefix do determine if secure on insecure gRPC connection should be used

**Example:**

* gRPC: `export OTEL_EXPORTER_OTLP_ENDPOINT="my-api-endpoint:443"`
* HTTP: `export OTEL_EXPORTER_OTLP_ENDPOINT="https://my-api-endpoint/"`
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be safer to set

Suggested change
* HTTP: `export OTEL_EXPORTER_OTLP_ENDPOINT="https://my-api-endpoint/"`
* HTTP: `export OTEL_EXPORTER_OTLP_ENDPOINT="https://my-api-endpoint"`


### `OTEL_EXPORTER_OTLP_ENDPOINT`

A base endpoint URL for any signal type, with an optionall-specified port
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
A base endpoint URL for any signal type, with an optionall-specified port
A base endpoint URL for any signal type, with an optionally-specified port


### `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`

Endpoint URL for trace data only, with an optionall-specified port number. Must
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Endpoint URL for trace data only, with an optionall-specified port number. Must
Endpoint URL for trace data only, with an optionally-specified port number. Must


### `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT`

Endpoint URL for trace data only, with an optionall-specified port number. Must
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Endpoint URL for trace data only, with an optionall-specified port number. Must
Endpoint URL for trace data only, with an optionally-specified port number. Must


### `OTEL_EXPORTER_OTLP_LOGS_ENDPOINT`

Endpoint URL for trace data only, with an optionall-specified port number. Must
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Endpoint URL for trace data only, with an optionall-specified port number. Must
Endpoint URL for trace data only, with an optionally-specified port number. Must

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.

3 participants