kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.54k stars 8.26k forks source link

Allow to enable OpenTelemetry tracing on individual ingress resources #10966

Open sergk-mambu opened 9 months ago

sergk-mambu commented 9 months ago

Currently if OpenTelemetry is disabled globally via the ConfigMap option enable-opentelemetry: "false" and enabled only in one ingress resource via the annotation nginx.ingress.kubernetes.io/enable-opentelemetry: "true" then OpenTelemetry tracing becomes implicitly enabled for all other ingress resources. It happens because in otel_ngx module tracing is enabled by default. So as soon as the module is loaded then tracing is enabled for all ingress instances (all servers in nginx config). The only way to make it enabled on one ingress instance is to add the annotation nginx.ingress.kubernetes.io/enable-opentelemetry: "false" to all other ingress instances which is cumbersome.

Previously with OpenTracing it worked fine because tracing was disabled by default in that module.

I propose to add here in nginx.tmpl a config option opentelemetry off; the next line after the option opentelemetry_config .... In this case it will mimic old OpenTracing behaviour, i.e. OpenTelemetry will be disabled in the http scope of nginx and then enabled per server/location if either global ConfigMap option or local annotation are enabled.

k8s-ci-robot commented 9 months ago

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
angelbarrera92 commented 8 months ago

I'm interested too. It doesn't make sense in the current configuration, if it is enabled by default, why does the doc mention that is possible to enable ingress per ingress? Doc, with the current behavior, must mention the opposite, how to offboard ingresses from opentelemetry.

ulvish commented 8 months ago

I have the same issue. Regardless of Global value in configmap enable-opentelemetry set to true or false, adding annotation to a single ingress resource, enables Open Telemetry for all ingress resources.

kalingaru48 commented 8 months ago

These OTEL annotations do not work as described in the doc https://kubernetes.github.io/ingress-nginx/user-guide/third-party-addons/opentelemetry/

I used nginx.ingress.kubernetes.io/enable-opentelemetry: "true" and then OTEL traces come from the all routes. With nginx.ingress.kubernetes.io/enable-opentelemetry: "false" added to a ingress route I still see traces from that route as well.

lerminou commented 3 months ago

I'm in this case too. I want to activate the telemetry for a single ingress, not the whole cluster

EDIT: based on the informations from @sergk-mambu , I have a workaround to set the opentelemetry: off in the http-snippet This way it works, the telemetry plugin is only enabled for my ingress location, but I aggre, this should be the default value

lieberlois commented 3 months ago

@lerminou Could you share that snippet? 😄

Overall: +1 for this issue!

lerminou commented 3 months ago

@lieberlois , just simply in the configmap:

apiVersion: v1
data:
  http-snippet: 
    opentelemetry off;

because the http-snippet is loaded at the end of the virtual host, it overrides the default value. https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/template/nginx.tmpl#L535

lieberlois commented 2 months ago

@lerminou Seems to work as a workaround - however, this can't be the official solution. Are you aware of any updates on this topic?

cardboardpig commented 5 days ago

While the work around works, you need at least one ingress with the annotation otherwise nginx will refuse to load its config while the snippet is present.