-
Notifications
You must be signed in to change notification settings - Fork 2k
Add OpenTelemetry support #7642
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
base: main
Are you sure you want to change the base?
Conversation
3f111b2
to
8c598c6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7642 +/- ##
==========================================
+ Coverage 52.29% 52.47% +0.18%
==========================================
Files 90 90
Lines 21472 21550 +78
==========================================
+ Hits 11228 11308 +80
+ Misses 9774 9772 -2
Partials 470 470 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
22e590f
to
4cf0f62
Compare
25f53b4
to
bee7750
Compare
8ad0ac7
to
bee7750
Compare
✅ Deploy Preview will be available once build job completes!
|
01535d0
to
1d581b5
Compare
714a273
to
406b1b8
Compare
Signed-off-by: Haywood Shannon <[email protected]> Signed-off-by: Haywood Shannon <[email protected]>
Signed-off-by: Haywood Shannon <[email protected]> Signed-off-by: Haywood Shannon <[email protected]>
2aade5c
to
93b48df
Compare
@@ -230,6 +230,11 @@ If you encounter the error `error [emerg] 13#13: "zone_sync" directive is duplic | |||
{{<bootstrap-table "table table-striped table-bordered table-responsive">}} | |||
|ConfigMap Key | Description | Default | Example | | |||
| ---| ---| ---| --- | | |||
|*otel-exporter-endpoint* | OTLP/gRPC endpoint that will accept [OpenTelemetry](https://opentelemetry.io) data. Set `otel-trace-in-http` to *True* to enable OpenTelemetry at global level. Example: *"https://otel-collector:4317"*. Note: requires the Ingress Controller image with OpenTelemetry module. | | | |
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 can specify N/A
as defaults for the ones without
|*otel-exporter-header-name* | The name of a custom HTTP header to add to telemetry export request. Example: *"X-custom-header"*. `otel-exporter-endpoint` and `otel-exporter-header-value` required. | | | | ||
|*otel-exporter-header-value* | The value of a custom HTTP header to add to telemetry export request.Example: *"custom-value"*. `otel-exporter-endpoint` and `otel-exporter-header-name` required. | | | | ||
|*otel-service-name* | Sets the `service.name` attribute of the OTel resource. Example: *"nginx-ingress-controller:nginx"*. `otel-exporter-endpoint` required. | | | | ||
| *otel-trace-in-http* | Enables [OpenTelemetry](https://opentelemetry.io) globally (for all Ingress, VirtualServer and VirtualServerRoute resources). Set this to *False* to enable OpenTelemetry for individual routes with snippets. `otel-exporter-endpoint` required. | *False* | | | ||
|*opentracing* | Removed in v5.0.0. Enables [OpenTracing](https://opentracing.io) globally (for all Ingress, VirtualServer and VirtualServerRoute resources). Note: requires the Ingress Controller image with OpenTracing module and a tracer. See the [docs]({{< relref "/installation/integrations/opentracing.md" >}}) for more information. | *False* | | |
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.
since this is more of a replacement, should we remove opentracing entries now? @shaun-nx
@@ -5,7 +5,7 @@ | |||
"image": "ubi-8-plus-nap", | |||
"type": "plus", | |||
"nap_modules": "waf", | |||
"marker": "appprotect_waf_policies_allow", | |||
"marker": "'appprotect_waf_policies_allow or otel'", |
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.
Imo we should create a new entry for Otel like OIDC, mixing it with other unrelated markers is a bit confusing especially since the configuration is controlled using configmap only
@@ -1022,7 +1022,7 @@ def get_nginx_template_conf(v1: CoreV1Api, ingress_namespace, ic_pod_name=None) | |||
if ic_pod_name is None: | |||
ic_pod_name = get_first_pod_name(v1, ingress_namespace) | |||
file_path = "/etc/nginx/nginx.conf" | |||
return get_file_contents(v1, file_path, ic_pod_name, ingress_namespace) | |||
return get_file_contents(v1, file_path, ic_pod_name, ingress_namespace, print_log) |
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.
do we need to pass print_log
as default is true?
@@ -0,0 +1,10 @@ | |||
# syntax=docker/dockerfile:1.8 |
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.
need to update docker-updater as it looks for Dockerfile.ubi
https://github.com/nginx/kubernetes-ingress/actions/runs/15278931054/job/42972620331#step:3:37
cc @pdabelf5
Proposed changes
Add otel support for NGINX Ingress Controller OSS and Plus
Added config map keys:
Checklist
Before creating a PR, run through this checklist and mark each as complete.