Update Telemetry Job with GRPC endpoint

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,13 @@ 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 |
+|`controller.telemetryReporting.secure` | If set to false, the option [otlptracegrpc.WithInsecure()](https://pkg.go.dev/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc#WithInsecure) will be set. | true |
+|`controller.telemetryReporting.endpoint` | GRPC endpoint to send data to. | "oss-dev.edge.df.f5.com:443" |
 |`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

@@ -263,7 +263,9 @@ 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}}
+- -telemetry-reporting-endpoint={{ .Values.controller.telemetryReporting.endpoint}}
+- -telemetry-reporting-secure={{ .Values.controller.telemetryReporting.secure}}
 {{- end -}}
 
 {{/*

charts/nginx-ingress/values.yaml

@@ -487,8 +487,16 @@ controller:
   ## Enable dynamic reloading of certificates
   enableSSLDynamicReload: true
 
-  ## Enable telemetry reporting
-  enableTelemetryReporting: true
+  ## Configure telemetry reporting options
+  telemetryReporting:
+    ## Enable telemetry reporting
+    enable: true
+
+    ## If set to false, the option otlptracegrpc.WithInsecure() will be set
+    secure: true
+
+    ## GRPC endpoint to send data to
+    endpoint: "oss-dev.edge.df.f5.com:443"
 
 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"
@@ -203,8 +201,9 @@ 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.")
+	enableTelemetryReporting   = flag.Bool("enable-telemetry-reporting", true, "Enable gathering and reporting of product related telemetry.")
+	telemetryReportingEndpoint = flag.String("telemetry-reporting-endpoint", "dev-oss.edge.df.f5.com:443", "Set the GRPC endpoint to send data to.")
+	telemetryReportingSecure   = flag.Bool("telemetry-reporting-secure", true, "If set to `false`, the `otlptracegrpc.WithInsecure()` option will be set for the exporter.")
 
 	startupCheckFn func() error
 )
@@ -389,12 +388,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
@@ -500,17 +493,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

@@ -204,7 +204,8 @@ func main() {
 		IsIPV6Disabled:               *disableIPV6,
 		WatchNamespaceLabel:          *watchNamespaceLabel,
 		EnableTelemetryReporting:     *enableTelemetryReporting,
-		TelemetryReportingPeriod:     *telemetryReportingPeriod,
+		TelemetryReportingEndpoint:   *telemetryReportingEndpoint,
+		TelemetryReportingSecure:     *telemetryReportingSecure,
 		NICVersion:                   version,
 	}
 

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

@@ -25,7 +25,10 @@ import (
 	"sync"
 	"time"
 
+	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
+
 	"github.com/nginxinc/kubernetes-ingress/internal/telemetry"
+	telemetryExporter "github.com/nginxinc/telemetry-exporter/pkg/telemetry"
 
 	"github.com/nginxinc/kubernetes-ingress/pkg/apis/dos/v1beta1"
 	"golang.org/x/exp/maps"
@@ -211,7 +214,8 @@ type NewLoadBalancerControllerInput struct {
 	IsIPV6Disabled               bool
 	WatchNamespaceLabel          string
 	EnableTelemetryReporting     bool
-	TelemetryReportingPeriod     string
+	TelemetryReportingEndpoint   string
+	TelemetryReportingSecure     bool
 	NICVersion                   string
 }
 
@@ -347,16 +351,34 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc
 
 	// NIC Telemetry Reporting
 	if input.EnableTelemetryReporting {
+		providerOptions := []otlptracegrpc.Option{
+			otlptracegrpc.WithEndpoint(input.TelemetryReportingEndpoint),
+			// 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",
+			}),
+		}
+
+		if !input.TelemetryReportingSecure {
+			providerOptions = append(providerOptions, otlptracegrpc.WithInsecure())
+		}
+
+		exporter, _ := telemetryExporter.NewExporter(
+			telemetryExporter.ExporterConfig{
+				SpanProvider: telemetryExporter.CreateOTLPSpanProvider(providerOptions...),
+			},
+		)
 		collectorConfig := telemetry.CollectorConfig{
 			K8sClientReader:       input.KubeClient,
 			CustomK8sClientReader: input.ConfClient,
-			Period:                5 * time.Second,
+			Period:                24 * time.Hour,
 			Configurator:          lbc.configurator,
 			Version:               input.NICVersion,
 		}
 		lbc.telemetryChan = make(chan struct{})
 		collector, err := telemetry.NewCollector(
 			collectorConfig,
+			telemetry.WithExporter(exporter),
 		)
 		if err != nil {
 			glog.Fatalf("failed to initialize telemetry collector: %v", err)

internal/k8s/controller_test.go

@@ -3758,6 +3758,20 @@ func TestPreSyncSecrets(t *testing.T) {
 	}
 }
 
+//func TestNewTelemetryExporter(t *testing.T) {
+//	t.Parallel()
+//
+//	testCases := []struct {
+//		testCase string
+//		input NewLoadBalancerControllerInput
+//		expectedExporter telemetry.Exporter
+//	}{
+//		{
+//
+//		},
+//	}
+//}
+
 func TestNewTelemetryCollector(t *testing.T) {
 	t.Parallel()
 
@@ -3772,7 +3786,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

@@ -111,7 +111,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