-
Notifications
You must be signed in to change notification settings - Fork 452
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
Enable instrumentation injecting only core SDK config #1000
Conversation
|
README.md
Outdated
You can configure the OpenTelemetry SDK for applications which can't | ||
currently be autoinstrumented by using `inject-sdk` in place of (e.g.) | ||
`inject-python` or `inject-java`. This will inject environment variables | ||
like `OTEL_RESOURCE_ATTRIBUTES` but will not actually provide the SDK via an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
via an init container
This is an implementation detail and it will not be true for e.g. Golang instrumentation. therefore it should not be mentioned here.
name: sdk-only | ||
spec: | ||
env: | ||
- name: OTEL_TRACES_EXPORTER |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to remove all this OTEL env vars rely on the env vars derived from the spec.
ce9eb98
to
8cf48ac
Compare
@pavolloffay thanks for your feedback, I've amended the commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. just some nits around docs
README.md
Outdated
Follow the instructions in the Dockerfiles on how to build a custom container image. | ||
|
||
#### Inject OpenTelemetry SDK environment variables only | ||
|
||
You can configure the OpenTelemetry SDK for applications which can't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit we don't use line breaks in the readme.
README.md
Outdated
|
||
You can configure the OpenTelemetry SDK for applications which can't | ||
currently be autoinstrumented by using `inject-sdk` in place of (e.g.) | ||
`inject-python` or `inject-java`. This will inject environment variables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should mention as well reporter and sampling config.
Closes open-telemetry#990. Adds support for injecting SDK environment variables only, without injecting language-specific libraries or config. Usage: ```bash instrumentation.opentelemetry.io/inject-sdk: "true" ```
@pavolloffay thanks for reviewing. I've updated the readme and rebased. |
…#1000) Closes open-telemetry#990. Adds support for injecting SDK environment variables only, without injecting language-specific libraries or config. Usage: ```bash instrumentation.opentelemetry.io/inject-sdk: "true" ```
Closes #990.
Adds support for injecting SDK environment variables only,
without injecting language-specific libraries or config.
Usage:
instrumentation.opentelemetry.io/inject-sdk: "true"