From db74214edcd36d64497e1df331e0aa08c55ffa7c Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Mar 2024 16:59:33 +0000 Subject: [PATCH 01/12] Enable Telemetry Job to configure an endpoint for the exporter --- charts/nginx-ingress/README.md | 8 +++-- charts/nginx-ingress/templates/_helpers.tpl | 4 ++- charts/nginx-ingress/values.yaml | 12 +++++-- cmd/nginx-ingress/flags.go | 35 +++++---------------- cmd/nginx-ingress/main.go | 3 +- go.mod | 2 +- internal/k8s/controller.go | 25 +++++++++++++-- internal/k8s/controller_test.go | 15 ++++++++- internal/telemetry/collector.go | 2 +- 9 files changed, 66 insertions(+), 40 deletions(-) diff --git a/charts/nginx-ingress/README.md b/charts/nginx-ingress/README.md index 403c5efd1c..40c48d12fd 100644 --- a/charts/nginx-ingress/README.md +++ b/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 | diff --git a/charts/nginx-ingress/templates/_helpers.tpl b/charts/nginx-ingress/templates/_helpers.tpl index 4b65c23082..691fea2760 100644 --- a/charts/nginx-ingress/templates/_helpers.tpl +++ b/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 -}} {{/* diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index 075a4521a5..0df0155767 100644 --- a/charts/nginx-ingress/values.yaml +++ b/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. diff --git a/cmd/nginx-ingress/flags.go b/cmd/nginx-ingress/flags.go index 2d162b5e3c..a8f59b24b2 100644 --- a/cmd/nginx-ingress/flags.go +++ b/cmd/nginx-ingress/flags.go @@ -1,20 +1,17 @@ package main import ( - "errors" "flag" "fmt" + "github.com/golang/glog" + api_v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/validation" "net" "os" "regexp" "strconv" "strings" - "time" - - "github.com/golang/glog" - api_v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/util/validation" ) const ( @@ -203,8 +200,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 ) @@ -390,11 +388,6 @@ func validationChecks() { } } - 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 -} diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 67c389f3f9..d83d148d69 100644 --- a/cmd/nginx-ingress/main.go +++ b/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, } diff --git a/go.mod b/go.mod index 16cec18873..0a6b89f774 100644 --- a/go.mod +++ b/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 diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index b54cc855c5..413812c6af 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -19,6 +19,7 @@ package k8s import ( "context" "fmt" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "net" "strconv" "strings" @@ -26,6 +27,7 @@ import ( "time" "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 +213,8 @@ type NewLoadBalancerControllerInput struct { IsIPV6Disabled bool WatchNamespaceLabel string EnableTelemetryReporting bool - TelemetryReportingPeriod string + TelemetryReportingEndpoint string + TelemetryReportingSecure bool NICVersion string } @@ -347,16 +350,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) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index cb8b30f274..5fc1201b7f 100644 --- a/internal/k8s/controller_test.go +++ b/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{ diff --git a/internal/telemetry/collector.go b/internal/telemetry/collector.go index a98fea9c72..5357c38ecb 100644 --- a/internal/telemetry/collector.go +++ b/internal/telemetry/collector.go @@ -93,7 +93,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", data) + glog.V(3).Infof("Telemetry data collected: %+v", data) } // BuildReport takes context and builds report from gathered telemetry data. From 7e115b2bc693851eec8bae3c6f857370c22ae5c1 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Mar 2024 17:02:54 +0000 Subject: [PATCH 02/12] gofumpt --- cmd/nginx-ingress/flags.go | 10 +++++----- internal/k8s/controller.go | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/nginx-ingress/flags.go b/cmd/nginx-ingress/flags.go index a8f59b24b2..3077f85032 100644 --- a/cmd/nginx-ingress/flags.go +++ b/cmd/nginx-ingress/flags.go @@ -3,15 +3,16 @@ package main import ( "flag" "fmt" - "github.com/golang/glog" - api_v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/util/validation" "net" "os" "regexp" "strconv" "strings" + + "github.com/golang/glog" + api_v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/validation" ) const ( @@ -387,7 +388,6 @@ func validationChecks() { glog.Fatalf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel) } } - } // validateNamespaceNames validates the namespaces are in the correct format diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 413812c6af..da68c38144 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -19,13 +19,14 @@ package k8s import ( "context" "fmt" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "net" "strconv" "strings" "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" From 0544d0714d677fe70bb97cbf2efe94b87268344b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 12 Mar 2024 17:07:46 +0000 Subject: [PATCH 03/12] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- internal/telemetry/data.avdl | 24 +++++++++---------- .../telemetry/data_attributes_generated.go | 6 ++--- .../nicresourcecounts_attributes_generated.go | 6 ++--- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/telemetry/data.avdl b/internal/telemetry/data.avdl index 7d72cc6330..8ce86fdee4 100644 --- a/internal/telemetry/data.avdl +++ b/internal/telemetry/data.avdl @@ -7,40 +7,40 @@ /** The time our edge ingested the event */ long ingestTime; - + /** ProjectName is the name of the project. */ string? ProjectName = null; - + /** ProjectVersion is the version of the project. */ string? ProjectVersion = null; - + /** ProjectArchitecture is the architecture of the project. For example, "amd64". */ string? ProjectArchitecture = null; - + /** ClusterID is the unique id of the Kubernetes cluster where the project is installed. It is the UID of the `kube-system` Namespace. */ string? ClusterID = null; - + /** ClusterVersion is the Kubernetes version of the cluster. */ string? ClusterVersion = null; - + /** ClusterPlatform is the Kubernetes platform of the cluster. */ string? ClusterPlatform = null; - + /** InstallationID is the unique id of the project installation in the cluster. */ string? InstallationID = null; - + /** ClusterNodeCount is the number of nodes in the cluster. */ long? ClusterNodeCount = null; - + /** VirtualServer is the number of VirtualServer managed by the Ingress Controller. */ long? VirtualServers = null; - + /** VirtualServerRoutes is the number of VirtualServerRoutes managed by the Ingress Controller. */ long? VirtualServerRoutes = null; - + /** TransportServers is the number of TransportServers managed by the Ingress Controller. */ long? TransportServers = null; - + } } diff --git a/internal/telemetry/data_attributes_generated.go b/internal/telemetry/data_attributes_generated.go index 0a68fb9b04..5503ddcf69 100644 --- a/internal/telemetry/data_attributes_generated.go +++ b/internal/telemetry/data_attributes_generated.go @@ -7,9 +7,9 @@ This is a generated file. DO NOT EDIT. import ( "go.opentelemetry.io/otel/attribute" - + ngxTelemetry "github.com/nginxinc/telemetry-exporter/pkg/telemetry" - + ) func (d *Data) Attributes() []attribute.KeyValue { @@ -17,7 +17,7 @@ func (d *Data) Attributes() []attribute.KeyValue { attrs = append(attrs, d.Data.Attributes()...) attrs = append(attrs, d.NICResourceCounts.Attributes()...) - + return attrs } diff --git a/internal/telemetry/nicresourcecounts_attributes_generated.go b/internal/telemetry/nicresourcecounts_attributes_generated.go index cf10cf3f93..725c60972d 100644 --- a/internal/telemetry/nicresourcecounts_attributes_generated.go +++ b/internal/telemetry/nicresourcecounts_attributes_generated.go @@ -7,9 +7,9 @@ This is a generated file. DO NOT EDIT. import ( "go.opentelemetry.io/otel/attribute" - + ngxTelemetry "github.com/nginxinc/telemetry-exporter/pkg/telemetry" - + ) func (d *NICResourceCounts) Attributes() []attribute.KeyValue { @@ -18,7 +18,7 @@ func (d *NICResourceCounts) Attributes() []attribute.KeyValue { attrs = append(attrs, attribute.Int64("VirtualServers", d.VirtualServers)) attrs = append(attrs, attribute.Int64("VirtualServerRoutes", d.VirtualServerRoutes)) attrs = append(attrs, attribute.Int64("TransportServers", d.TransportServers)) - + return attrs } From 94d22433edf98d5ba63b5a4319edc705334cfa18 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Mar 2024 17:19:30 +0000 Subject: [PATCH 04/12] Fix conflit in attributes --- internal/telemetry/data_attributes_generated.go | 5 +---- internal/telemetry/nicresourcecounts_attributes_generated.go | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/internal/telemetry/data_attributes_generated.go b/internal/telemetry/data_attributes_generated.go index 5503ddcf69..9fbc1610e6 100644 --- a/internal/telemetry/data_attributes_generated.go +++ b/internal/telemetry/data_attributes_generated.go @@ -1,5 +1,5 @@ - package telemetry + /* This is a generated file. DO NOT EDIT. */ @@ -7,9 +7,7 @@ This is a generated file. DO NOT EDIT. import ( "go.opentelemetry.io/otel/attribute" - ngxTelemetry "github.com/nginxinc/telemetry-exporter/pkg/telemetry" - ) func (d *Data) Attributes() []attribute.KeyValue { @@ -18,7 +16,6 @@ func (d *Data) Attributes() []attribute.KeyValue { attrs = append(attrs, d.Data.Attributes()...) attrs = append(attrs, d.NICResourceCounts.Attributes()...) - return attrs } diff --git a/internal/telemetry/nicresourcecounts_attributes_generated.go b/internal/telemetry/nicresourcecounts_attributes_generated.go index 725c60972d..1c85cd5542 100644 --- a/internal/telemetry/nicresourcecounts_attributes_generated.go +++ b/internal/telemetry/nicresourcecounts_attributes_generated.go @@ -1,5 +1,5 @@ - package telemetry + /* This is a generated file. DO NOT EDIT. */ @@ -7,9 +7,7 @@ This is a generated file. DO NOT EDIT. import ( "go.opentelemetry.io/otel/attribute" - ngxTelemetry "github.com/nginxinc/telemetry-exporter/pkg/telemetry" - ) func (d *NICResourceCounts) Attributes() []attribute.KeyValue { @@ -19,7 +17,6 @@ func (d *NICResourceCounts) Attributes() []attribute.KeyValue { attrs = append(attrs, attribute.Int64("VirtualServerRoutes", d.VirtualServerRoutes)) attrs = append(attrs, attribute.Int64("TransportServers", d.TransportServers)) - return attrs } From 709c3e6713bffc2b346da14e3e4bd07145ab3b03 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 13 Mar 2024 08:25:20 +0000 Subject: [PATCH 05/12] Remove undefined function --- cmd/nginx-ingress/flags_test.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/cmd/nginx-ingress/flags_test.go b/cmd/nginx-ingress/flags_test.go index 07a32a3f7f..1715d2a3e8 100644 --- a/cmd/nginx-ingress/flags_test.go +++ b/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) - } - } -} From 11acce507370263274188a48c211bf2c609ea1c2 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 13 Mar 2024 14:28:07 +0000 Subject: [PATCH 06/12] Move creation of exporter to exporter.go and update public docs --- cmd/nginx-ingress/flags.go | 2 +- .../command-line-arguments.md | 24 +++++++++++++++ .../installing-nic/installation-with-helm.md | 19 +++++++----- internal/k8s/controller.go | 25 +++++----------- internal/telemetry/exporter.go | 29 +++++++++++++++++++ 5 files changed, 72 insertions(+), 27 deletions(-) diff --git a/cmd/nginx-ingress/flags.go b/cmd/nginx-ingress/flags.go index 3077f85032..c7522bca93 100644 --- a/cmd/nginx-ingress/flags.go +++ b/cmd/nginx-ingress/flags.go @@ -202,7 +202,7 @@ 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.") - telemetryReportingEndpoint = flag.String("telemetry-reporting-endpoint", "dev-oss.edge.df.f5.com:443", "Set the GRPC endpoint to send data to.") + telemetryReportingEndpoint = flag.String("telemetry-reporting-endpoint", "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 diff --git a/docs/content/configuration/global-configuration/command-line-arguments.md b/docs/content/configuration/global-configuration/command-line-arguments.md index 93e14eea02..a5e7750b6e 100644 --- a/docs/content/configuration/global-configuration/command-line-arguments.md +++ b/docs/content/configuration/global-configuration/command-line-arguments.md @@ -535,3 +535,27 @@ Used to activate or deactivate lazy loading for SSL Certificates. The default value is `true`. + +### -enable-telemetry-reporting + +Enable gathering and reporting of product related telemetry. + +The default value is `true`. + + + +### -telemetry-reporting-endpoint + +Sets the GRPC endpoint to send data to. + +The default value is `oss.edge.df.f5.com:443`. + + + +### -telemetry-reporting-secure + +If set to `false`, the [otlptracegrpc.WithInsecure()](https://pkg.go.dev/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc#WithInsecure) option will be set for the exporter. + +The default value is `true`. + + diff --git a/docs/content/installation/installing-nic/installation-with-helm.md b/docs/content/installation/installing-nic/installation-with-helm.md index 7eda35bb3b..34f38763b2 100644 --- a/docs/content/installation/installing-nic/installation-with-helm.md +++ b/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 ``` - 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 ``` - Copy the key=value under ```Selector```, such as: + Copy the key=value under ```Selector```, such as: ```shell Selector: app=-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,9 @@ 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 | +| **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 | diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index da68c38144..bbfc952136 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -25,11 +25,7 @@ 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" @@ -351,23 +347,16 @@ 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", - }), + exporterCfg := telemetry.ExporterCfg{ + Endpoint: input.TelemetryReportingEndpoint, + Secure: input.TelemetryReportingSecure, } - if !input.TelemetryReportingSecure { - providerOptions = append(providerOptions, otlptracegrpc.WithInsecure()) - } + exporter, err := telemetry.NewExporter(exporterCfg) - exporter, _ := telemetryExporter.NewExporter( - telemetryExporter.ExporterConfig{ - SpanProvider: telemetryExporter.CreateOTLPSpanProvider(providerOptions...), - }, - ) + if err != nil { + glog.Fatalf("failed to initialize telemetry exporter: %v", err) + } collectorConfig := telemetry.CollectorConfig{ K8sClientReader: input.KubeClient, CustomK8sClientReader: input.ConfClient, diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index 3969fa3852..dd1e62d0ca 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -3,6 +3,7 @@ package telemetry import ( "context" "fmt" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "io" tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry" @@ -24,6 +25,34 @@ 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 + Secure bool +} + +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", + }), + } + + if !cfg.Secure { + providerOptions = append(providerOptions, otlptracegrpc.WithInsecure()) + } + + 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 From a8a4bdba64bc8a455ba79a2d286941d1f0d9c45f Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 13 Mar 2024 14:40:09 +0000 Subject: [PATCH 07/12] gofumpt --- internal/k8s/controller.go | 1 - internal/telemetry/exporter.go | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index bbfc952136..8bea47f086 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -353,7 +353,6 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc } exporter, err := telemetry.NewExporter(exporterCfg) - if err != nil { glog.Fatalf("failed to initialize telemetry exporter: %v", err) } diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index dd1e62d0ca..404ba8afbb 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -3,9 +3,10 @@ package telemetry import ( "context" "fmt" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "io" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" + tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry" ) @@ -31,6 +32,7 @@ type ExporterCfg struct { Secure bool } +// NewExporter creates an Exporter with the provided ExporterCfg. func NewExporter(cfg ExporterCfg) (Exporter, error) { providerOptions := []otlptracegrpc.Option{ otlptracegrpc.WithEndpoint(cfg.Endpoint), From 20ff03fa799960bd15dbe136f7100f244f20717f Mon Sep 17 00:00:00 2001 From: Venktesh Date: Thu, 14 Mar 2024 12:08:22 +0000 Subject: [PATCH 08/12] disable telemetry from tests --- tests/suite/utils/resources_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/suite/utils/resources_utils.py b/tests/suite/utils/resources_utils.py index cb2ab4c087..bd9be37b08 100644 --- a/tests/suite/utils/resources_utils.py +++ b/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) From a9349d42ce097e40e9e1a7c1b38c55b09586a901 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 14 Mar 2024 12:18:44 +0000 Subject: [PATCH 09/12] Set endpoint for exporter --- charts/nginx-ingress/README.md | 2 -- charts/nginx-ingress/templates/_helpers.tpl | 2 -- charts/nginx-ingress/values.yaml | 6 ------ cmd/nginx-ingress/flags.go | 4 +--- cmd/nginx-ingress/main.go | 2 -- .../command-line-arguments.md | 16 ---------------- .../installing-nic/installation-with-helm.md | 2 -- internal/k8s/controller.go | 7 ++----- internal/telemetry/exporter.go | 5 ----- 9 files changed, 3 insertions(+), 43 deletions(-) diff --git a/charts/nginx-ingress/README.md b/charts/nginx-ingress/README.md index 453b44df18..05f72f11ee 100644 --- a/charts/nginx-ingress/README.md +++ b/charts/nginx-ingress/README.md @@ -471,8 +471,6 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont |`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 | -|`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 | diff --git a/charts/nginx-ingress/templates/_helpers.tpl b/charts/nginx-ingress/templates/_helpers.tpl index 2579f4b39c..6bd2bd1783 100644 --- a/charts/nginx-ingress/templates/_helpers.tpl +++ b/charts/nginx-ingress/templates/_helpers.tpl @@ -278,8 +278,6 @@ Build the args for the service binary. - -enable-latency-metrics={{ .Values.controller.enableLatencyMetrics }} - -ssl-dynamic-reload={{ .Values.controller.enableSSLDynamicReload }} - -enable-telemetry-reporting={{ .Values.controller.telemetryReporting.enable}} -- -telemetry-reporting-endpoint={{ .Values.controller.telemetryReporting.endpoint}} -- -telemetry-reporting-secure={{ .Values.controller.telemetryReporting.secure}} {{- if .Values.nginxAgent.enable }} - -agent=true - -agent-instance-group={{ default (include "nginx-ingress.controller.fullname" .) .Values.nginxAgent.instanceGroup }} diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index 31b50dec83..8b09d8be87 100644 --- a/charts/nginx-ingress/values.yaml +++ b/charts/nginx-ingress/values.yaml @@ -492,12 +492,6 @@ controller: ## 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. create: true diff --git a/cmd/nginx-ingress/flags.go b/cmd/nginx-ingress/flags.go index 777de04e61..2ad5ce294d 100644 --- a/cmd/nginx-ingress/flags.go +++ b/cmd/nginx-ingress/flags.go @@ -204,9 +204,7 @@ 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.") - telemetryReportingEndpoint = flag.String("telemetry-reporting-endpoint", "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.") + enableTelemetryReporting = flag.Bool("enable-telemetry-reporting", true, "Enable gathering and reporting of product related telemetry.") startupCheckFn func() error ) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 9509e5e4e0..0c03ce266e 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -210,8 +210,6 @@ func main() { IsIPV6Disabled: *disableIPV6, WatchNamespaceLabel: *watchNamespaceLabel, EnableTelemetryReporting: *enableTelemetryReporting, - TelemetryReportingEndpoint: *telemetryReportingEndpoint, - TelemetryReportingSecure: *telemetryReportingSecure, NICVersion: version, } diff --git a/docs/content/configuration/global-configuration/command-line-arguments.md b/docs/content/configuration/global-configuration/command-line-arguments.md index 29cee1d4d3..33c13fa7c9 100644 --- a/docs/content/configuration/global-configuration/command-line-arguments.md +++ b/docs/content/configuration/global-configuration/command-line-arguments.md @@ -544,22 +544,6 @@ The default value is `true`. -### -telemetry-reporting-endpoint - -Sets the GRPC endpoint to send data to. - -The default value is `oss.edge.df.f5.com:443`. - - - -### -telemetry-reporting-secure - -If set to `false`, the [otlptracegrpc.WithInsecure()](https://pkg.go.dev/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc#WithInsecure) option will be set for the exporter. - -The default value is `true`. - - - ### -agent Enable NGINX Agent which can used with `-enable-app-protect` to send events to Security Monitoring. diff --git a/docs/content/installation/installing-nic/installation-with-helm.md b/docs/content/installation/installing-nic/installation-with-helm.md index 634630251e..8d6a7eafb4 100644 --- a/docs/content/installation/installing-nic/installation-with-helm.md +++ b/docs/content/installation/installing-nic/installation-with-helm.md @@ -434,8 +434,6 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont | **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 | -| **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 | diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 8bea47f086..ad57f095cc 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -210,8 +210,6 @@ type NewLoadBalancerControllerInput struct { IsIPV6Disabled bool WatchNamespaceLabel string EnableTelemetryReporting bool - TelemetryReportingEndpoint string - TelemetryReportingSecure bool NICVersion string } @@ -347,9 +345,9 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc // NIC Telemetry Reporting if input.EnableTelemetryReporting { + lbc.telemetryChan = make(chan struct{}) exporterCfg := telemetry.ExporterCfg{ - Endpoint: input.TelemetryReportingEndpoint, - Secure: input.TelemetryReportingSecure, + Endpoint: "oss.edge.df.f5.com:443", } exporter, err := telemetry.NewExporter(exporterCfg) @@ -363,7 +361,6 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc Configurator: lbc.configurator, Version: input.NICVersion, } - lbc.telemetryChan = make(chan struct{}) collector, err := telemetry.NewCollector( collectorConfig, telemetry.WithExporter(exporter), diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index 404ba8afbb..31a955228c 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -29,7 +29,6 @@ func (e *StdoutExporter) Export(_ context.Context, data tel.Exportable) error { // ExporterCfg is a configuration struct for an Exporter. type ExporterCfg struct { Endpoint string - Secure bool } // NewExporter creates an Exporter with the provided ExporterCfg. @@ -42,10 +41,6 @@ func NewExporter(cfg ExporterCfg) (Exporter, error) { }), } - if !cfg.Secure { - providerOptions = append(providerOptions, otlptracegrpc.WithInsecure()) - } - exporter, err := tel.NewExporter( tel.ExporterConfig{ SpanProvider: tel.CreateOTLPSpanProvider(providerOptions...), From 214adec0fd89ac71921cbda8105cfa82161fbc06 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 14 Mar 2024 12:19:31 +0000 Subject: [PATCH 10/12] Create channel only if collector is successfully created --- internal/k8s/controller.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index ad57f095cc..3b41f2df50 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -345,7 +345,7 @@ 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", } @@ -369,6 +369,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc glog.Fatalf("failed to initialize telemetry collector: %v", err) } lbc.telemetryCollector = collector + lbc.telemetryChan = make(chan struct{}) } return lbc From 1bde61314a2fab6a91ce90a9108d20ede23222a9 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 14 Mar 2024 13:57:04 +0000 Subject: [PATCH 11/12] Update json schema for helm values --- charts/nginx-ingress/values.schema.json | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/charts/nginx-ingress/values.schema.json b/charts/nginx-ingress/values.schema.json index 53c9b7198d..0e1613a59f 100644 --- a/charts/nginx-ingress/values.schema.json +++ b/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": [ From 2084ec03a8eb373cd5ca5c62d5f738d7f7c41b91 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 14 Mar 2024 14:04:14 +0000 Subject: [PATCH 12/12] Fix json schema error --- charts/nginx-ingress/values.schema.json | 2 +- internal/k8s/controller_test.go | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/charts/nginx-ingress/values.schema.json b/charts/nginx-ingress/values.schema.json index 0e1613a59f..3ccd63bead 100644 --- a/charts/nginx-ingress/values.schema.json +++ b/charts/nginx-ingress/values.schema.json @@ -1430,7 +1430,7 @@ "examples": [ true ] - }, + } } } }, diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 5fc1201b7f..50725ea714 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -3758,20 +3758,6 @@ 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()