Update Telemetry Job with GRPC endpoint (#5237)

charts/nginx-ingress/README.md

@@ -387,7 +387,7 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
 |`controller.replicaCount` | The number of replicas of the Ingress Controller deployment. | 1 |
 |`controller.ingressClass.name` | A class of the Ingress Controller. An IngressClass resource with the name equal to the class must be deployed. Otherwise, the Ingress Controller will fail to start. The Ingress Controller only processes resources that belong to its class - i.e. have the "ingressClassName" field resource equal to the class. The Ingress Controller processes all the VirtualServer/VirtualServerRoute/TransportServer resources that do not have the "ingressClassName" field for all versions of Kubernetes. | nginx |
 |`controller.ingressClass.create` | Creates a new IngressClass object with the name `controller.ingressClass.name`. Set to `false` to use an existing ingressClass created using `kubectl` with the same name. If you use `helm upgrade`, do not change the values from the previous release as helm will delete IngressClass objects managed by helm. If you are upgrading from a release earlier than 3.4.3, do not set the value to false. | true |
-|`controller.ingressClass.setAsDefaultIngress` | New Ingresses without an `"ingressClassName"` field specified will be assigned the class specified in `controller.ingressClass.name`. Requires `controller.ingressClass.create`.  | false |
+|`controller.ingressClass.setAsDefaultIngress` | New Ingresses without an `"ingressClassName"` field specified will be assigned the class specified in `controller.ingressClass.name`. Requires `controller.ingressClass.create`. | false |
 |`controller.watchNamespace` | Comma separated list of namespaces the Ingress Controller should watch for resources. By default the Ingress Controller watches all namespaces. Mutually exclusive with `controller.watchNamespaceLabel`. Please note that if configuring multiple namespaces using the Helm cli `--set` option, the string needs to wrapped in double quotes and the commas escaped using a backslash - e.g. `--set controller.watchNamespace="default\,nginx-ingress"`. | "" |
 |`controller.watchNamespaceLabel` | Configures the Ingress Controller to watch only those namespaces with label foo=bar. By default the Ingress Controller watches all namespaces. Mutually exclusive with `controller.watchNamespace`. | "" |
 |`controller.watchSecretNamespace` | Comma separated list of namespaces the Ingress Controller should watch for resources of type Secret. If this arg is not configured, the Ingress Controller watches the same namespaces for all resources. See `controller.watchNamespace` and `controller.watchNamespaceLabel`. Please note that if configuring multiple namespaces using the Helm cli `--set` option, the string needs to wrapped in double quotes and the commas escaped using a backslash - e.g. `--set controller.watchSecretNamespace="default\,nginx-ingress"`. | "" |
@@ -466,11 +466,11 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
 |`controller.podDisruptionBudget.maxUnavailable` | The number of Ingress Controller pods that can be unavailable. This is a mutually exclusive setting with "minAvailable". | 0 |
 |`controller.strategy` | Specifies the strategy used to replace old Pods with new ones. Docs for [Deployment update strategy](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy) and [Daemonset update strategy](https://kubernetes.io/docs/tasks/manage-daemon/update-daemon-set/#daemonset-update-strategy) | {} |
 |`controller.disableIPV6` | Disable IPV6 listeners explicitly for nodes that do not support the IPV6 stack. | false |
-|`controller.defaultHTTPListenerPort`  | Sets the port for the HTTP `default_server` listener. | 80 |
+|`controller.defaultHTTPListenerPort` | Sets the port for the HTTP `default_server` listener. | 80 |
 |`controller.defaultHTTPSListenerPort` | Sets the port for the HTTPS `default_server` listener. | 443 |
 |`controller.readOnlyRootFilesystem` | Configure root filesystem as read-only and add volumes for temporary data. Three major releases after 3.5.x this argument will be moved permanently to the `controller.securityContext` section. | false |
 |`controller.enableSSLDynamicReload` | Enable lazy loading for SSL Certificates. | true |
-|`controller.enableTelemetryReporting` | Enable telemetry reporting. | true |
+|`controller.telemetryReporting.enable` | Enable telemetry reporting. | true |
 |`rbac.create` | Configures RBAC. | true |
 |`prometheus.create` | Expose NGINX or NGINX Plus metrics in the Prometheus format. | true |
 |`prometheus.port` | Configures the port to scrape the metrics. | 9113 |

charts/nginx-ingress/templates/_helpers.tpl

@@ -277,7 +277,7 @@ Build the args for the service binary.
 - -ready-status-port={{ .Values.controller.readyStatus.port }}
 - -enable-latency-metrics={{ .Values.controller.enableLatencyMetrics }}
 - -ssl-dynamic-reload={{ .Values.controller.enableSSLDynamicReload }}
-- -enable-telemetry-reporting={{ .Values.controller.enableTelemetryReporting}}
+- -enable-telemetry-reporting={{ .Values.controller.telemetryReporting.enable}}
 {{- if .Values.nginxAgent.enable }}
 - -agent=true
 - -agent-instance-group={{ default (include "nginx-ingress.controller.fullname" .) .Values.nginxAgent.instanceGroup }}

charts/nginx-ingress/values.schema.json

@@ -1417,13 +1417,21 @@
             true
           ]
         },
-        "enableTelemetryReporting": {
-          "type": "boolean",
-          "default": true,
-          "title": "Enable telemetry reporting",
-          "examples": [
-            true
-          ]
+        "telemetryReporting": {
+          "type": "object",
+          "default": {},
+          "title": "Configure telemetry reporting options",
+          "required": [],
+          "properties": {
+            "enable": {
+              "type": "boolean",
+              "default": true,
+              "title": "Enable telemetry reporting",
+              "examples": [
+                true
+              ]
+            }
+          }
         }
       },
       "examples": [

charts/nginx-ingress/values.yaml

@@ -487,8 +487,10 @@ controller:
   ## Enable dynamic reloading of certificates
   enableSSLDynamicReload: true
 
-  ## Enable telemetry reporting
-  enableTelemetryReporting: true
+  ## Configure telemetry reporting options
+  telemetryReporting:
+    ## Enable telemetry reporting
+    enable: true
 
 rbac:
   ## Configures RBAC.

cmd/nginx-ingress/flags.go

@@ -1,15 +1,13 @@
 package main
 
 import (
-	"errors"
 	"flag"
 	"fmt"
 	"net"
 	"os"
 	"regexp"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/golang/glog"
 	api_v1 "k8s.io/api/core/v1"
@@ -207,7 +205,6 @@ var (
 	enableDynamicSSLReload = flag.Bool(dynamicSSLReloadParam, true, "Enable reloading of SSL Certificates without restarting the NGINX process.")
 
 	enableTelemetryReporting = flag.Bool("enable-telemetry-reporting", true, "Enable gathering and reporting of product related telemetry.")
-	telemetryReportingPeriod = flag.String("telemetry-reporting-period", "24h", "Sets a telemetry reporting period.")
 
 	startupCheckFn func() error
 )
@@ -396,12 +393,6 @@ func validationChecks() {
 			glog.Fatalf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel)
 		}
 	}
-
-	if telemetryReportingPeriod != nil {
-		if err := validateReportingPeriod(*telemetryReportingPeriod); err != nil {
-			glog.Fatalf("Invalid value for telemetry-reporting-period: %v", err)
-		}
-	}
 }
 
 // validateNamespaceNames validates the namespaces are in the correct format
@@ -507,17 +498,3 @@ func validateLocation(location string) error {
 	}
 	return nil
 }
-
-// validateReportingPeriod checks if the reporting period parameter can be parsed.
-//
-// This function will be deprecated in NIC v3.5. It is used only for demo and testing purpose.
-func validateReportingPeriod(period string) error {
-	duration, err := time.ParseDuration(period)
-	if err != nil {
-		return err
-	}
-	if duration.Minutes() < 1 {
-		return errors.New("invalid reporting period, expected minimum 1m")
-	}
-	return nil
-}

cmd/nginx-ingress/flags_test.go

@@ -172,27 +172,3 @@ func TestValidateNamespaces(t *testing.T) {
 		}
 	}
 }
-
-func TestValidateReportingPeriodWithInvalidInput(t *testing.T) {
-	t.Parallel()
-
-	periods := []string{"", "-1", "1x", "abc", "-", "30s", "10ms", "0h"}
-	for _, p := range periods {
-		err := validateReportingPeriod(p)
-		if err == nil {
-			t.Errorf("want error on invalid period %s, got nil", p)
-		}
-	}
-}
-
-func TestValidateReportingPeriodWithValidInput(t *testing.T) {
-	t.Parallel()
-
-	periods := []string{"1m", "1h", "24h"}
-	for _, p := range periods {
-		err := validateReportingPeriod(p)
-		if err != nil {
-			t.Error(err)
-		}
-	}
-}

cmd/nginx-ingress/main.go

@@ -210,7 +210,6 @@ func main() {
 		IsIPV6Disabled:               *disableIPV6,
 		WatchNamespaceLabel:          *watchNamespaceLabel,
 		EnableTelemetryReporting:     *enableTelemetryReporting,
-		TelemetryReportingPeriod:     *telemetryReportingPeriod,
 		NICVersion:                   version,
 	}
 

docs/content/configuration/global-configuration/command-line-arguments.md

@@ -536,6 +536,14 @@ The default value is `true`.
 
 <a name="cmdoption-ssl-dynamic-reload"></a>
 
+### -enable-telemetry-reporting
+
+Enable gathering and reporting of product related telemetry.
+
+The default value is `true`.
+
+<a name="cmdoption-enable-telemetry-reporting"></a>
+
 ### -agent
 
 Enable NGINX Agent which can used with `-enable-app-protect` to send events to Security Monitoring.

docs/content/installation/installing-nic/installation-with-helm.md

@@ -22,10 +22,10 @@ h2 {
 - A [Kubernetes Version Supported by the Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/technical-specifications/#supported-kubernetes-versions)
 - Helm 3.0+.
 - If you’d like to use NGINX Plus:
-  - To pull from the F5 Container registry, configure a docker registry secret using your JWT token from the MyF5 portal by following the instructions from [here](https://docs.nginx.com/nginx-ingress-controller/installation/nic-images/using-the-jwt-token-docker-secret). Make sure to specify the secret using `controller.serviceAccount.imagePullSecretName` or `controller.serviceAccount.imagePullSecretsNames` parameter.
-  - Alternatively, pull an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/nic-images/pulling-ingress-controller-image" >}}).
-  - Alternatively, you can build an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/building-nginx-ingress-controller.md" >}}).
-  - Update the `controller.image.repository` field of the `values-plus.yaml` accordingly.
+    - To pull from the F5 Container registry, configure a docker registry secret using your JWT token from the MyF5 portal by following the instructions from [here](https://docs.nginx.com/nginx-ingress-controller/installation/nic-images/using-the-jwt-token-docker-secret). Make sure to specify the secret using `controller.serviceAccount.imagePullSecretName` or `controller.serviceAccount.imagePullSecretsNames` parameter.
+    - Alternatively, pull an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/nic-images/pulling-ingress-controller-image" >}}).
+    - Alternatively, you can build an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/building-nginx-ingress-controller.md" >}}).
+    - Update the `controller.image.repository` field of the `values-plus.yaml` accordingly.
 - If you’d like to use App Protect DoS, please install App Protect DoS Arbitrator [helm chart](https://github.com/nginxinc/nap-dos-arbitrator-helm-chart). Make sure to install in the same namespace as the NGINX Ingress Controller. Note that if you install multiple NGINX Ingress Controllers in the same namespace, they will need to share the same Arbitrator because it is not possible to install more than one Arbitrator in a single namespace.
 
 ## CRDs
@@ -204,7 +204,7 @@ The steps you should follow depend on the Helm release name:
     kubectl describe deployments -n <namespace>
     ```
 
-    Copy the key=value under `Selector`, such as:
+   Copy the key=value under `Selector`, such as:
 
     ```shell
     Selector: app=nginx-ingress-nginx-ingress
@@ -225,7 +225,7 @@ The steps you should follow depend on the Helm release name:
     --set controller.name=""
     --set fullnameOverride="nginx-ingress-nginx-ingress"
     ```
-    It could look as follows:
+   It could look as follows:
 
     ```shell
     helm upgrade nginx-ingress oci://ghcr.io/nginxinc/charts/nginx-ingress --version 0.19.0 --set controller.kind=deployment/daemonset --set controller.nginxplus=false/true --set controller.image.pullPolicy=Always --set serviceNameOverride="nginx-ingress-nginx-ingress" --set controller.name="" --set fullnameOverride="nginx-ingress-nginx-ingress" -f values.yaml
@@ -249,7 +249,7 @@ The steps you should follow depend on the Helm release name:
     kubectl describe deployment/daemonset -n <namespace>
     ```
 
-    Copy the key=value under ```Selector```, such as:
+   Copy the key=value under ```Selector```, such as:
 
     ```shell
     Selector: app=<helm_release_name>-nginx-ingress
@@ -272,7 +272,7 @@ The steps you should follow depend on the Helm release name:
     --set controller.name=""
     ```
 
-    It could look as follows:
+   It could look as follows:
 
     ```shell
     helm upgrade test-release oci://ghcr.io/nginxinc/charts/nginx-ingress --version 0.19.0 --set controller.kind=deployment/daemonset --set controller.nginxplus=false/true --set controller.image.pullPolicy=Always --set serviceNameOverride="test-release-nginx-ingress" --set controller.name="" -f values.yaml
@@ -433,6 +433,7 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
 | **controller.defaultHTTPSListenerPort**  | Sets the port for the HTTPS `default_server` listener. | 443 |
 | **controller.readOnlyRootFilesystem** | Configure root filesystem as read-only and add volumes for temporary data. Three major releases after 3.5.x this argument will be moved permanently to the `controller.securityContext` section.  | false |
 | **controller.enableSSLDynamicReload** | Enable lazy loading for SSL Certificates. | true |
+| **controller.telemetryReporting.enable** | Enable telemetry reporting. | true |
 | **rbac.create** | Configures RBAC. | true |
 | **prometheus.create** | Expose NGINX or NGINX Plus metrics in the Prometheus format. | true |
 | **prometheus.port** | Configures the port to scrape the metrics. | 9113 |

go.mod

@@ -21,6 +21,7 @@ require (
 	github.com/spiffe/go-spiffe/v2 v2.1.7
 	github.com/stretchr/testify v1.9.0
 	go.opentelemetry.io/otel v1.24.0
+	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.24.0
 	golang.org/x/exp v0.0.0-20231226003508-02704c960a9b
 	k8s.io/api v0.29.2
 	k8s.io/apimachinery v0.29.2
@@ -101,7 +102,6 @@ require (
 	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.46.1 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.24.0 // indirect
-	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.24.0 // indirect
 	go.opentelemetry.io/otel/metric v1.24.0 // indirect
 	go.opentelemetry.io/otel/sdk v1.24.0 // indirect
 	go.opentelemetry.io/otel/trace v1.24.0 // indirect

internal/k8s/controller.go

@@ -27,7 +27,6 @@ import (
 	"time"
 
 	"github.com/nginxinc/kubernetes-ingress/internal/telemetry"
-
 	"github.com/nginxinc/kubernetes-ingress/pkg/apis/dos/v1beta1"
 	"golang.org/x/exp/maps"
 
@@ -213,7 +212,6 @@ type NewLoadBalancerControllerInput struct {
 	IsIPV6Disabled               bool
 	WatchNamespaceLabel          string
 	EnableTelemetryReporting     bool
-	TelemetryReportingPeriod     string
 	NICVersion                   string
 }
 
@@ -349,24 +347,34 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc
 
 	// NIC Telemetry Reporting
 	if input.EnableTelemetryReporting {
-		lbc.telemetryChan = make(chan struct{})
+		exporterCfg := telemetry.ExporterCfg{
+			Endpoint: "oss.edge.df.f5.com:443",
+		}
 
+		exporter, err := telemetry.NewExporter(exporterCfg)
+		if err != nil {
+			glog.Fatalf("failed to initialize telemetry exporter: %v", err)
+		}
 		collectorConfig := telemetry.CollectorConfig{
 			K8sClientReader:       input.KubeClient,
 			CustomK8sClientReader: input.ConfClient,
-			Period:                5 * time.Second,
+			Period:                24 * time.Hour,
 			Configurator:          lbc.configurator,
 			Version:               input.NICVersion,
 			PodNSName: types.NamespacedName{
 				Namespace: os.Getenv("POD_NAMESPACE"),
 				Name:      os.Getenv("POD_NAME"),
 			},
 		}
-		collector, err := telemetry.NewCollector(collectorConfig)
+		collector, err := telemetry.NewCollector(
+			collectorConfig,
+			telemetry.WithExporter(exporter),
+		)
 		if err != nil {
 			glog.Fatalf("failed to initialize telemetry collector: %v", err)
 		}
 		lbc.telemetryCollector = collector
+		lbc.telemetryChan = make(chan struct{})
 	}
 
 	return lbc

internal/k8s/controller_test.go

@@ -3772,7 +3772,6 @@ func TestNewTelemetryCollector(t *testing.T) {
 			input: NewLoadBalancerControllerInput{
 				KubeClient:               fake.NewSimpleClientset(),
 				EnableTelemetryReporting: true,
-				TelemetryReportingPeriod: "24h",
 			},
 			expectedCollector: telemetry.Collector{
 				Config: telemetry.CollectorConfig{

internal/telemetry/collector.go

@@ -116,7 +116,7 @@ func (c *Collector) Collect(ctx context.Context) {
 	if err != nil {
 		glog.Errorf("Error exporting telemetry data: %v", err)
 	}
-	glog.V(3).Infof("Exported telemetry data: %+v", nicData)
+	glog.V(3).Infof("Telemetry data collected: %+v", nicData)
 }
 
 // Report holds collected NIC telemetry data. It is the package internal

internal/telemetry/exporter.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"io"
 
+	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
+
 	tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry"
 )
 
@@ -24,6 +26,30 @@ func (e *StdoutExporter) Export(_ context.Context, data tel.Exportable) error {
 	return nil
 }
 
+// ExporterCfg is a configuration struct for an Exporter.
+type ExporterCfg struct {
+	Endpoint string
+}
+
+// NewExporter creates an Exporter with the provided ExporterCfg.
+func NewExporter(cfg ExporterCfg) (Exporter, error) {
+	providerOptions := []otlptracegrpc.Option{
+		otlptracegrpc.WithEndpoint(cfg.Endpoint),
+		// This header option will be removed when https://github.com/nginxinc/telemetry-exporter/issues/41 is resolved.
+		otlptracegrpc.WithHeaders(map[string]string{
+			"X-F5-OTEL": "GRPC",
+		}),
+	}
+
+	exporter, err := tel.NewExporter(
+		tel.ExporterConfig{
+			SpanProvider: tel.CreateOTLPSpanProvider(providerOptions...),
+		},
+	)
+
+	return exporter, err
+}
+
 // Data holds collected telemetry data.
 //
 //go:generate go run -tags=generator github.com/nginxinc/telemetry-exporter/cmd/generator -type Data -scheme -scheme-protocol=NICProductTelemetry -scheme-df-datatype=nic-product-telemetry -scheme-namespace=ingress.nginx.com

tests/suite/utils/resources_utils.py

@@ -1182,7 +1182,10 @@ def create_ingress_controller(v1: CoreV1Api, apps_v1_api: AppsV1Api, cli_argumen
     dep["spec"]["template"]["spec"]["containers"][0]["image"] = cli_arguments["image"]
     dep["spec"]["template"]["spec"]["containers"][0]["imagePullPolicy"] = cli_arguments["image-pull-policy"]
     dep["spec"]["template"]["spec"]["containers"][0]["args"].extend(
-        ["-default-server-tls-secret=$(POD_NAMESPACE)/default-server-secret"]
+        [
+            f"-default-server-tls-secret=$(POD_NAMESPACE)/default-server-secret",
+            f"-enable-telemetry-reporting=false",
+        ]
     )
     if args is not None:
         dep["spec"]["template"]["spec"]["containers"][0]["args"].extend(args)