-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Pass through Datadog environment variables to Nginx when EnableOpentracing+DatadogCollectorHost set #4372
Comments
On further investigation, it looks like all of these configuration options can also be set in Some of those options are exposed by the existing ingress-nginx/internal/ingress/controller/nginx.go Lines 1055 to 1060 in 771fc9f
The correct approach is probably to populate the remaining options here, and not deal in environment variables. |
Hang on, is |
Correct. In some cases, it's only via environment variable instead of config file or tracer options (code that initializes a tracer). No strong reason for it, but the intention was to make things easier for users and projects consuming the tracer. Eg. avoiding the need to add new code. In this case though, it's making things harder. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@fejta-bot: Closing this issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Is this a request for help? (If yes, you should use our troubleshooting guide and community support channels, see https://kubernetes.io/docs/tasks/debug-application-cluster/troubleshooting/.):
What keywords did you search in NGINX Ingress controller issues before filing this one? (If you have found any duplicates, you should instead reply there.):
Is this a BUG REPORT or FEATURE REQUEST? (choose one): Feature
NGINX Ingress controller version: 0.26.0
Kubernetes version (use
kubectl version
):Environment:
uname -a
):What happened:
After #4371 merged, the
dd-opentracing-cpp
module detects several environment variables and configures tracer behaviour based on their values.DD_ENV
DD_GLOBAL_ANALYTICS_SAMPLE_RATE
andDD_TRACE_ANALYTICS_ENABLED
Setting those environment variables in the
dd-opentracing-cpp
requires setting them in the ingress-nginx pod (very reasonable) and requires whitelisting them for passthrough in a custom nginx template or main-snippet likeWhat you expected to happen:
Generate nginx.conf correctly without repeating config in multiple places.
How to reproduce it (as minimally and precisely as possible):
Build ingress-nginx from master after #4371 merged, set environment variable
DD_TRACE_ANALYTICS_ENABLED=true
on the ingress-nginx Pod, don't see trace-analytics enabled indd-opentracing-cpp
because nginx doesn't pass it through by default. Read http://nginx.org/en/docs/ngx_core_module.html#env and define a custommain-snippet
as above. Be happier.Anything else we need to know:
I'm going to make a PR for this, unless someone beats me to it.
The text was updated successfully, but these errors were encountered: