From 04bd4e95f0fdb633223d757eeb51152c15a6761e Mon Sep 17 00:00:00 2001 From: AlexFenlon Date: Tue, 4 Jun 2024 08:50:06 +0100 Subject: [PATCH] Add Granular Services Counts to Telemetry (#5627) --- docs/content/overview/product-telemetry.md | 5 +- internal/configs/configurator.go | 100 ---- internal/telemetry/cluster.go | 16 + internal/telemetry/cluster_test.go | 188 ++++++ internal/telemetry/collector.go | 146 +++-- internal/telemetry/collector_test.go | 562 +++++------------- internal/telemetry/data.avdl | 13 +- internal/telemetry/exporter.go | 10 +- .../nicresourcecounts_attributes_generated.go | 5 +- 9 files changed, 448 insertions(+), 597 deletions(-) diff --git a/docs/content/overview/product-telemetry.md b/docs/content/overview/product-telemetry.md index 753a8477e3..d6ed533cd1 100644 --- a/docs/content/overview/product-telemetry.md +++ b/docs/content/overview/product-telemetry.md @@ -34,7 +34,10 @@ These are the data points collected and reported by NGINX Ingress Controller: - **TransportServers** The number of TransportServer resources managed by NGINX Ingress Controller. - **Replicas** Number of Deployment replicas, or Daemonset instances. - **Secrets** Number of Secret resources managed by NGINX Ingress Controller. -- **Services** Number of Services referenced by VirtualServers, VirtualServerRoutes, TransportServers and Ingresses. +- **ClusterIPServices** Number of ClusterIP Services managed by NGINX Ingress Controller. +- **NodePortServices** Number of NodePort Services managed by NGINX Ingress Controller. +- **LoadBalancerServices** Number of LoadBalancer Services managed by NGINX Ingress Controller. +- **ExternalNameServices** Number of ExternalName Services managed by NGINX Ingress Controller. - **RegularIngressCount** The number of Regular Ingress resources managed by NGINX Ingress Controller. - **MasterIngressCount** The number of Master Ingress resources managed by NGINX Ingress Controller. - **MinionIngressCount** The number of Minion Ingress resources managed by NGINX Ingress Controller. diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 4b84efb11f..080f597ceb 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -1604,106 +1604,6 @@ func (cnf *Configurator) getMinionIngressAnnotations(annotationSet map[string]bo return annotationSet } -// GetServiceCount returns the total number of unique services referenced by Ingresses, VS's, VSR's, and TS's -func (cnf *Configurator) GetServiceCount() int { - setOfUniqueServices := make(map[string]bool) - cnf.addVSAndVSRServicesToSet(setOfUniqueServices) - cnf.addTSServicesToSet(setOfUniqueServices) - cnf.addIngressesServicesToSet(setOfUniqueServices) - return len(setOfUniqueServices) -} - -// addVSAndVSRServicesToSet adds services from VirtualServers and VirtualServerRoutes to the set -func (cnf *Configurator) addVSAndVSRServicesToSet(set map[string]bool) { - for _, vs := range cnf.virtualServers { - ns := vs.VirtualServer.Namespace - for _, upstream := range vs.VirtualServer.Spec.Upstreams { - svc := upstream.Service - addServiceToSet(set, ns, svc) - - if upstream.Backup != "" { - addServiceToSet(set, ns, upstream.Backup) - } - - if upstream.HealthCheck != nil && upstream.HealthCheck.GRPCService != "" { - addServiceToSet(set, ns, upstream.HealthCheck.GRPCService) - } - } - - for _, vsr := range vs.VirtualServerRoutes { - ns := vsr.Namespace - for _, upstream := range vsr.Spec.Upstreams { - svc := upstream.Service - addServiceToSet(set, ns, svc) - - if upstream.Backup != "" { - addServiceToSet(set, ns, upstream.Backup) - } - - if upstream.HealthCheck != nil && upstream.HealthCheck.GRPCService != "" { - addServiceToSet(set, ns, upstream.HealthCheck.GRPCService) - } - } - } - } -} - -// addTSServicesToSet adds services from TransportServers to the set -func (cnf *Configurator) addTSServicesToSet(set map[string]bool) { - for _, ts := range cnf.transportServers { - ns := ts.TransportServer.Namespace - for _, upstream := range ts.TransportServer.Spec.Upstreams { - svc := upstream.Service - addServiceToSet(set, ns, svc) - - if upstream.Backup != "" { - addServiceToSet(set, ns, upstream.Backup) - } - - } - } -} - -// addIngressesServicesToSet adds services from Ingresses to the set -func (cnf *Configurator) addIngressesServicesToSet(set map[string]bool) { - for _, ing := range cnf.ingresses { - cnf.addIngressServicesToSet(ing, set) - } - for _, mergeIngs := range cnf.mergeableIngresses { - cnf.addIngressServicesToSet(mergeIngs.Master, set) - for _, minion := range mergeIngs.Minions { - cnf.addIngressServicesToSet(minion, set) - } - } -} - -// addIngressServicesToSet processes a single ingress and adds its services to the set -func (cnf *Configurator) addIngressServicesToSet(ing *IngressEx, set map[string]bool) { - if ing == nil || ing.Ingress == nil { - return - } - ns := ing.Ingress.Namespace - if ing.Ingress.Spec.DefaultBackend != nil && ing.Ingress.Spec.DefaultBackend.Service != nil { - svc := ing.Ingress.Spec.DefaultBackend.Service.Name - addServiceToSet(set, ns, svc) - } - for _, rule := range ing.Ingress.Spec.Rules { - if rule.HTTP != nil { - for _, path := range rule.HTTP.Paths { - if path.Backend.Service != nil { - svc := path.Backend.Service.Name - addServiceToSet(set, ns, svc) - } - } - } - } -} - -// Helper function to add services to the set -func addServiceToSet(set map[string]bool, ns string, svc string) { - set[fmt.Sprintf("%s/%s", ns, svc)] = true -} - // GetVirtualServerCounts returns the total count of // VirtualServer and VirtualServerRoute resources that are handled by the Ingress Controller func (cnf *Configurator) GetVirtualServerCounts() (int, int) { diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 8125f098b0..28219116b6 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -212,6 +212,22 @@ func (c *Collector) InstallationFlags() []string { return c.Config.InstallationFlags } +// ServiceCounts returns a map of service names and their counts in the Kubernetes cluster. +func (c *Collector) ServiceCounts() (map[string]int, error) { + serviceCounts := make(map[string]int) + + services, err := c.Config.K8sClientReader.CoreV1().Services("").List(context.Background(), metaV1.ListOptions{}) + if err != nil { + return nil, err + } + + for _, service := range services.Items { + serviceCounts[string(service.Spec.Type)]++ + } + + return serviceCounts, nil +} + // lookupPlatform takes a string representing a K8s PlatformID // retrieved from a cluster node and returns a string // representing the platform name. diff --git a/internal/telemetry/cluster_test.go b/internal/telemetry/cluster_test.go index fb88ba7004..788730fc0f 100644 --- a/internal/telemetry/cluster_test.go +++ b/internal/telemetry/cluster_test.go @@ -459,6 +459,80 @@ func TestGetInstallationFlags(t *testing.T) { } } +func TestGetServices(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + config telemetry.CollectorConfig + want map[string]int + }{ + { + name: "OneClusterIP", + config: telemetry.CollectorConfig{ + K8sClientReader: newTestClientset(defaultNS, kubeNS, clusterIPService), + }, + want: map[string]int{ + "ClusterIP": 1, + }, + }, + { + name: "MultipleClusterIPs", + config: telemetry.CollectorConfig{ + K8sClientReader: newTestClientset(defaultNS, kubeNS, clusterIPService, clusterIPService2), + }, + want: map[string]int{ + "ClusterIP": 2, + }, + }, + { + name: "MultipleExternalNamesAndNodePort", + config: telemetry.CollectorConfig{ + K8sClientReader: newTestClientset(defaultNS, kubeNS, externalNameService, externalNameService2, nodePortService), + }, + want: map[string]int{ + "ExternalName": 2, + "NodePort": 1, + }, + }, + { + name: "MultipleServices", + config: telemetry.CollectorConfig{ + K8sClientReader: newTestClientset(defaultNS, kubeNS, externalNameService, externalNameService2, nodePortService, nodePortService2, clusterIPService2, clusterIPService, loadBalancerService), + }, + want: map[string]int{ + "ClusterIP": 2, + "ExternalName": 2, + "NodePort": 2, + "LoadBalancer": 1, + }, + }, + { + name: "noServices", + config: telemetry.CollectorConfig{ + K8sClientReader: newTestClientset(defaultNS, kubeNS), + }, + want: map[string]int{}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + c, err := telemetry.NewCollector(tc.config) + if err != nil { + t.Fatal(err) + } + + got, err := c.ServiceCounts() + if err != nil { + t.Fatal(err) + } + + if !cmp.Equal(tc.want, got) { + t.Error(cmp.Diff(tc.want, got)) + } + }) + } +} + // newTestCollectorForClusterWithNodes returns a telemetry collector configured // to simulate collecting data on a cluser with provided nodes. func newTestCollectorForClusterWithNodes(t *testing.T, nodes ...runtime.Object) *telemetry.Collector { @@ -671,6 +745,18 @@ var ( }, Spec: apiCoreV1.NamespaceSpec{}, } + + defaultNS = &apiCoreV1.Namespace{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Namespace", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "default", + UID: "329766ff-5d78-4c9e-8736-7fesd1f2e937", + }, + Spec: apiCoreV1.NamespaceSpec{}, + } ) // Cloud providers' nodes for testing ProviderID lookups. @@ -937,3 +1023,105 @@ var ( Data: map[string][]byte{}, } ) + +// Services for testing +var ( + clusterIPService = &apiCoreV1.Service{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "coffee-svc", + Namespace: "default", + }, + Spec: apiCoreV1.ServiceSpec{ + Type: "ClusterIP", + }, + Status: apiCoreV1.ServiceStatus{}, + } + clusterIPService2 = &apiCoreV1.Service{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "coffee-svc2", + Namespace: "default", + }, + Spec: apiCoreV1.ServiceSpec{ + Type: "ClusterIP", + }, + Status: apiCoreV1.ServiceStatus{}, + } + nodePortService = &apiCoreV1.Service{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "tea-svc", + Namespace: "default", + }, + Spec: apiCoreV1.ServiceSpec{ + Type: "NodePort", + }, + Status: apiCoreV1.ServiceStatus{}, + } + nodePortService2 = &apiCoreV1.Service{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "tea-svc2", + Namespace: "default", + }, + Spec: apiCoreV1.ServiceSpec{ + Type: "NodePort", + }, + Status: apiCoreV1.ServiceStatus{}, + } + externalNameService = &apiCoreV1.Service{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "external-svc", + Namespace: "default", + }, + Spec: apiCoreV1.ServiceSpec{ + Type: "ExternalName", + }, + Status: apiCoreV1.ServiceStatus{}, + } + externalNameService2 = &apiCoreV1.Service{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "external-svc2", + Namespace: "default", + }, + Spec: apiCoreV1.ServiceSpec{ + Type: "ExternalName", + }, + Status: apiCoreV1.ServiceStatus{}, + } + loadBalancerService = &apiCoreV1.Service{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "default-svc", + Namespace: "default", + }, + Spec: apiCoreV1.ServiceSpec{ + Type: "LoadBalancer", + }, + Status: apiCoreV1.ServiceStatus{}, + } +) diff --git a/internal/telemetry/collector.go b/internal/telemetry/collector.go index a032f57e55..7c4c2dfdd1 100644 --- a/internal/telemetry/collector.go +++ b/internal/telemetry/collector.go @@ -129,7 +129,10 @@ func (c *Collector) Collect(ctx context.Context) { TransportServers: int64(report.TransportServers), Replicas: int64(report.NICReplicaCount), Secrets: int64(report.Secrets), - Services: int64(report.ServiceCount), + ClusterIPServices: int64(report.ClusterIPServices), + NodePortServices: int64(report.NodePortServices), + LoadBalancerServices: int64(report.LoadBalancerServices), + ExternalNameServices: int64(report.ExternalNameServices), RegularIngressCount: int64(report.RegularIngressCount), MasterIngressCount: int64(report.MasterIngressCount), MinionIngressCount: int64(report.MinionIngressCount), @@ -161,37 +164,40 @@ func (c *Collector) Collect(ctx context.Context) { // data structure used for decoupling types between the NIC `telemetry` // package and the imported `telemetry` exporter. type Report struct { - Name string - Version string - Architecture string - ClusterID string - ClusterVersion string - ClusterPlatform string - ClusterNodeCount int - InstallationID string - NICReplicaCount int - VirtualServers int - VirtualServerRoutes int - ServiceCount int - TransportServers int - Secrets int - RegularIngressCount int - MasterIngressCount int - MinionIngressCount int - IngressClassCount int - AccessControlCount int - RateLimitCount int - JWTAuthCount int - BasicAuthCount int - IngressMTLSCount int - EgressMTLSCount int - OIDCCount int - WAFCount int - GlobalConfiguration bool - IngressAnnotations []string - AppProtectVersion string - IsPlus bool - InstallationFlags []string + Name string + Version string + Architecture string + ClusterID string + ClusterVersion string + ClusterPlatform string + ClusterNodeCount int + InstallationID string + NICReplicaCount int + VirtualServers int + VirtualServerRoutes int + ClusterIPServices int + NodePortServices int + LoadBalancerServices int + ExternalNameServices int + TransportServers int + Secrets int + RegularIngressCount int + MasterIngressCount int + MinionIngressCount int + IngressClassCount int + AccessControlCount int + RateLimitCount int + JWTAuthCount int + BasicAuthCount int + IngressMTLSCount int + EgressMTLSCount int + OIDCCount int + WAFCount int + GlobalConfiguration bool + IngressAnnotations []string + AppProtectVersion string + IsPlus bool + InstallationFlags []string } // BuildReport takes context, collects telemetry data and builds the report. @@ -199,12 +205,10 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) { vsCount := 0 vsrCount := 0 tsCount := 0 - serviceCount := 0 if c.Config.Configurator != nil { vsCount, vsrCount = c.Config.Configurator.GetVirtualServerCounts() tsCount = c.Config.Configurator.GetTransportServerCounts() - serviceCount = c.Config.Configurator.GetServiceCount() } clusterID, err := c.ClusterID(ctx) @@ -269,37 +273,49 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) { installationFlags := c.InstallationFlags() + serviceCounts, err := c.ServiceCounts() + if err != nil { + glog.V(3).Infof("Unable to collect telemetry data: Service Counts: %v", err) + } + clusterIPServices := serviceCounts["ClusterIP"] + nodePortServices := serviceCounts["NodePort"] + loadBalancerServices := serviceCounts["LoadBalancer"] + externalNameServices := serviceCounts["ExternalName"] + return Report{ - Name: "NIC", - Version: c.Config.Version, - Architecture: runtime.GOARCH, - ClusterID: clusterID, - ClusterVersion: version, - ClusterPlatform: platform, - ClusterNodeCount: nodes, - InstallationID: installationID, - NICReplicaCount: replicas, - VirtualServers: vsCount, - VirtualServerRoutes: vsrCount, - ServiceCount: serviceCount, - TransportServers: tsCount, - Secrets: secretCount, - RegularIngressCount: regularIngressCount, - MasterIngressCount: masterIngressCount, - MinionIngressCount: minionIngressCount, - IngressClassCount: ingressClassCount, - AccessControlCount: accessControlCount, - RateLimitCount: rateLimitCount, - JWTAuthCount: jwtAuthCount, - BasicAuthCount: basicAuthCount, - IngressMTLSCount: ingressMTLSCount, - EgressMTLSCount: egressMTLSCount, - OIDCCount: oidcCount, - WAFCount: wafCount, - GlobalConfiguration: c.Config.GlobalConfiguration, - IngressAnnotations: ingressAnnotations, - AppProtectVersion: appProtectVersion, - IsPlus: isPlus, - InstallationFlags: installationFlags, + Name: "NIC", + Version: c.Config.Version, + Architecture: runtime.GOARCH, + ClusterID: clusterID, + ClusterVersion: version, + ClusterPlatform: platform, + ClusterNodeCount: nodes, + InstallationID: installationID, + NICReplicaCount: replicas, + VirtualServers: vsCount, + VirtualServerRoutes: vsrCount, + ClusterIPServices: clusterIPServices, + NodePortServices: nodePortServices, + LoadBalancerServices: loadBalancerServices, + ExternalNameServices: externalNameServices, + TransportServers: tsCount, + Secrets: secretCount, + RegularIngressCount: regularIngressCount, + MasterIngressCount: masterIngressCount, + MinionIngressCount: minionIngressCount, + IngressClassCount: ingressClassCount, + AccessControlCount: accessControlCount, + RateLimitCount: rateLimitCount, + JWTAuthCount: jwtAuthCount, + BasicAuthCount: basicAuthCount, + IngressMTLSCount: ingressMTLSCount, + EgressMTLSCount: egressMTLSCount, + OIDCCount: oidcCount, + WAFCount: wafCount, + GlobalConfiguration: c.Config.GlobalConfiguration, + IngressAnnotations: ingressAnnotations, + AppProtectVersion: appProtectVersion, + IsPlus: isPlus, + InstallationFlags: installationFlags, }, err } diff --git a/internal/telemetry/collector_test.go b/internal/telemetry/collector_test.go index e2ce8a9ae8..2383a992bc 100644 --- a/internal/telemetry/collector_test.go +++ b/internal/telemetry/collector_test.go @@ -748,7 +748,6 @@ func TestIngressCountReportsNumberOfDeployedIngresses(t *testing.T) { VirtualServerRoutes: 0, TransportServers: 0, RegularIngressCount: 1, - Services: 2, } td := telemetry.Data{ @@ -798,7 +797,6 @@ func TestMasterMinionIngressCountReportsNumberOfDeployedIngresses(t *testing.T) TransportServers: 0, MasterIngressCount: 1, MinionIngressCount: 2, - Services: 2, IngressAnnotations: []string{"nginx.org/mergeable-ingress-type"}, } @@ -1437,489 +1435,201 @@ func TestCountSecretsAddTwoSecretsAndDeleteOne(t *testing.T) { } } -func TestCountVirtualServersServices(t *testing.T) { +func TestCollectGetServices(t *testing.T) { t.Parallel() - testCases := []struct { - testName string - expectedTraceDataOnAdd telemetry.Report - expectedTraceDataOnDelete telemetry.Report - virtualServers []*configs.VirtualServerEx - deleteCount int + name string + config telemetry.CollectorConfig + want map[string]int }{ { - testName: "Create and delete 1 VirtualServer with 2 upstreams", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 2, + name: "OneClusterIP", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS, clusterIPService), + Version: telemetryNICData.ProjectVersion, }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, + want: map[string]int{ + "ClusterIP": 1, }, - virtualServers: []*configs.VirtualServerEx{ - { - VirtualServer: &conf_v1.VirtualServer{ - ObjectMeta: metaV1.ObjectMeta{ - Namespace: "ns-1", - Name: "coffee", - }, - Spec: conf_v1.VirtualServerSpec{ - Upstreams: []conf_v1.Upstream{ - { - Name: "coffee", - Service: "coffee-svc", - }, - { - Name: "coffee2", - Service: "coffee-svc2", - }, - }, - }, - }, - }, - }, - deleteCount: 1, }, { - testName: "Same service in 2 upstreams is only counted once", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 1, - }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, + name: "MultipleClusterIPs", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS, clusterIPService, clusterIPService2), + Version: telemetryNICData.ProjectVersion, }, - virtualServers: []*configs.VirtualServerEx{ - { - VirtualServer: &conf_v1.VirtualServer{ - ObjectMeta: metaV1.ObjectMeta{ - Namespace: "ns-1", - Name: "coffee", - }, - Spec: conf_v1.VirtualServerSpec{ - Upstreams: []conf_v1.Upstream{ - { - Name: "coffee", - Service: "same-svc", - }, - { - Name: "coffee2", - Service: "same-svc", - }, - }, - }, - }, - }, + want: map[string]int{ + "ClusterIP": 2, }, - deleteCount: 1, }, { - testName: "A backup service is counted in addition to the primary service", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 2, - }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, + name: "MultipleExternalNamesAndNodePort", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS, externalNameService, externalNameService2, nodePortService), + Version: telemetryNICData.ProjectVersion, }, - virtualServers: []*configs.VirtualServerEx{ - { - VirtualServer: &conf_v1.VirtualServer{ - ObjectMeta: metaV1.ObjectMeta{ - Namespace: "ns-1", - Name: "coffee", - }, - Spec: conf_v1.VirtualServerSpec{ - Upstreams: []conf_v1.Upstream{ - { - Name: "coffee", - Service: "same-svc", - Backup: "backup-service", - }, - }, - }, - }, - }, + want: map[string]int{ + "ExternalName": 2, + "NodePort": 1, }, - deleteCount: 1, }, { - testName: "A grpc service is counted in addition to the primary service and backup service", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 3, + name: "MultipleServices", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS, externalNameService, externalNameService2, nodePortService, nodePortService2, clusterIPService2, clusterIPService, loadBalancerService), + Version: telemetryNICData.ProjectVersion, }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, + want: map[string]int{ + "ClusterIP": 2, + "ExternalName": 2, + "NodePort": 2, + "LoadBalancer": 1, }, - virtualServers: []*configs.VirtualServerEx{ - { - VirtualServer: &conf_v1.VirtualServer{ - ObjectMeta: metaV1.ObjectMeta{ - Namespace: "ns-1", - Name: "coffee", - }, - Spec: conf_v1.VirtualServerSpec{ - Upstreams: []conf_v1.Upstream{ - { - Name: "coffee", - Service: "same-svc", - Backup: "backup-service", - HealthCheck: &conf_v1.HealthCheck{ - GRPCService: "grpc-service", - }, - }, - }, - }, - }, - }, + }, + { + name: "noServices", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS), + Version: telemetryNICData.ProjectVersion, }, - deleteCount: 1, + want: map[string]int{}, }, } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + buf := &bytes.Buffer{} + exp := &telemetry.StdoutExporter{Endpoint: buf} + cfg := tc.config - for _, test := range testCases { - configurator := newConfigurator(t) - - c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica), - Configurator: configurator, - Version: telemetryNICData.ProjectVersion, - SecretStore: newSecretStore(t), - }) - if err != nil { - t.Fatal(err) - } - c.Config.PodNSName = types.NamespacedName{ - Namespace: "nginx-ingress", - Name: "nginx-ingress", - } - - for _, vs := range test.virtualServers { - _, err := configurator.AddOrUpdateVirtualServer(vs) + c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) if err != nil { t.Fatal(err) } - } - - gotTraceDataOnAdd, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } + c.Collect(context.Background()) - if !cmp.Equal(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount)) - } + telData := tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ProjectArchitecture: telemetryNICData.ProjectArchitecture, + ClusterID: telemetryNICData.ClusterID, + ClusterVersion: telemetryNICData.ClusterVersion, + } + clusterIPServices := tc.want["ClusterIP"] + nodePortServices := tc.want["NodePort"] + loadBalancerServices := tc.want["LoadBalancer"] + externalNameServices := tc.want["ExternalName"] - for i := 0; i < test.deleteCount; i++ { - vs := test.virtualServers[i] - key := getResourceKey(vs.VirtualServer.Namespace, vs.VirtualServer.Name) - err := configurator.DeleteVirtualServer(key, false) - if err != nil { - t.Fatal(err) + nicResourceCounts := telemetry.NICResourceCounts{ + ClusterIPServices: int64(clusterIPServices), + NodePortServices: int64(nodePortServices), + LoadBalancerServices: int64(loadBalancerServices), + ExternalNameServices: int64(externalNameServices), } - } - gotTraceDataOnDelete, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } + td := telemetry.Data{ + Data: telData, + NICResourceCounts: nicResourceCounts, + } - if !cmp.Equal(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount)) - } + want := fmt.Sprintf("%+v", &td) + got := buf.String() + if !cmp.Equal(want, got) { + t.Error(cmp.Diff(want, got)) + } + }) } } -func TestCountTransportServersServices(t *testing.T) { +func TestCollectGetInvalidServices(t *testing.T) { t.Parallel() - testCases := []struct { - testName string - expectedTraceDataOnAdd telemetry.Report - expectedTraceDataOnDelete telemetry.Report - transportServers []*configs.TransportServerEx - deleteCount int + name string + config telemetry.CollectorConfig + want map[string]int }{ { - testName: "Create and delete 1 TransportServer with 2 upstreams", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 2, - }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, + name: "WantNoClusterIPServices", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS, clusterIPService), + Version: telemetryNICData.ProjectVersion, }, - transportServers: []*configs.TransportServerEx{ - { - TransportServer: &conf_v1.TransportServer{ - ObjectMeta: metaV1.ObjectMeta{ - Namespace: "ns-1", - Name: "coffee", - }, - Spec: conf_v1.TransportServerSpec{ - Action: &conf_v1.TransportServerAction{ - Pass: "coffee", - }, - Upstreams: []conf_v1.TransportServerUpstream{ - { - Name: "coffee", - Service: "coffee-svc", - }, - { - Name: "coffee2", - Service: "coffee-svc2", - }, - }, - }, - }, - }, + want: map[string]int{ + "ClusterIP": 0, }, - deleteCount: 1, }, { - testName: "Same service in 2 upstreams is only counted once", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 1, - }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, + name: "WantMultipleExternalNamesAndNodePort", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS, externalNameService2), + Version: telemetryNICData.ProjectVersion, }, - transportServers: []*configs.TransportServerEx{ - { - TransportServer: &conf_v1.TransportServer{ - ObjectMeta: metaV1.ObjectMeta{ - Namespace: "ns-1", - Name: "coffee", - }, - Spec: conf_v1.TransportServerSpec{ - Action: &conf_v1.TransportServerAction{ - Pass: "coffee", - }, - Upstreams: []conf_v1.TransportServerUpstream{ - { - Name: "coffee", - Service: "same-svc", - }, - { - Name: "coffee2", - Service: "same-svc", - }, - }, - }, - }, - }, + want: map[string]int{ + "ExternalName": 2, + "NodePort": 1, }, - deleteCount: 1, }, - } - - for _, test := range testCases { - configurator := newConfigurator(t) - - c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica), - Configurator: configurator, - Version: telemetryNICData.ProjectVersion, - SecretStore: newSecretStore(t), - }) - if err != nil { - t.Fatal(err) - } - c.Config.PodNSName = types.NamespacedName{ - Namespace: "nginx-ingress", - Name: "nginx-ingress", - } - - for _, ts := range test.transportServers { - _, err := configurator.AddOrUpdateTransportServer(ts) - if err != nil { - t.Fatal(err) - } - } - - gotTraceDataOnAdd, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } - - if !cmp.Equal(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount)) - } - - for i := 0; i < test.deleteCount; i++ { - ts := test.transportServers[i] - key := getResourceKey(ts.TransportServer.Namespace, ts.TransportServer.Name) - err := configurator.DeleteTransportServer(key) - if err != nil { - t.Fatal(err) - } - } - - gotTraceDataOnDelete, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } - - if !cmp.Equal(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount)) - } - } -} - -func TestCountIngressesServices(t *testing.T) { - t.Parallel() - - testCases := []struct { - testName string - expectedTraceDataOnAdd telemetry.Report - expectedTraceDataOnDelete telemetry.Report - ingress configs.IngressEx - deleteCount int - }{ { - testName: "Create and delete 1 Ingress with 2 services", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 2, + name: "WantManyServices", + config: telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(defaultNS, kubeNS, nodePortService2, clusterIPService2, clusterIPService, loadBalancerService), + Version: telemetryNICData.ProjectVersion, }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, + want: map[string]int{ + "ClusterIP": 2, + "ExternalName": 2, + "NodePort": 2, + "LoadBalancer": 2, }, - ingress: createCafeIngressEx(), - deleteCount: 1, }, } - for _, tc := range testCases { - test := tc - configurator := newConfigurator(t) - - c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica), - Configurator: configurator, - Version: telemetryNICData.ProjectVersion, - SecretStore: newSecretStore(t), - }) - if err != nil { - t.Fatal(err) - } - c.Config.PodNSName = types.NamespacedName{ - Namespace: "nginx-ingress", - Name: "nginx-ingress", - } - - _, err = configurator.AddOrUpdateIngress(&test.ingress) - if err != nil { - t.Fatal(err) - } - - gotTraceDataOnAdd, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } - - if !cmp.Equal(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount)) - } - - for i := 0; i < test.deleteCount; i++ { - ing := test.ingress + t.Run(tc.name, func(t *testing.T) { + buf := &bytes.Buffer{} + exp := &telemetry.StdoutExporter{Endpoint: buf} + cfg := tc.config - key := fmt.Sprintf("%s/%s", ing.Ingress.Namespace, ing.Ingress.Name) - err := configurator.DeleteIngress(key, false) + c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) if err != nil { t.Fatal(err) } - } - - if err != nil { - t.Fatal(err) - } - - gotTraceDataOnDelete, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } - - if !cmp.Equal(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount)) - } - } -} - -func TestCountMergeableIngressesServices(t *testing.T) { - t.Parallel() - - testCases := []struct { - testName string - expectedTraceDataOnAdd telemetry.Report - expectedTraceDataOnDelete telemetry.Report - ingress *configs.MergeableIngresses - deleteCount int - }{ - { - testName: "Create and delete 1 MergeableIngress with 2 services", - expectedTraceDataOnAdd: telemetry.Report{ - ServiceCount: 2, - }, - expectedTraceDataOnDelete: telemetry.Report{ - ServiceCount: 0, - }, - ingress: createMergeableCafeIngress(), - deleteCount: 1, - }, - } - - for _, tc := range testCases { - test := tc - - configurator := newConfigurator(t) - - c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica), - Configurator: configurator, - Version: telemetryNICData.ProjectVersion, - SecretStore: newSecretStore(t), - }) - if err != nil { - t.Fatal(err) - } - c.Config.PodNSName = types.NamespacedName{ - Namespace: "nginx-ingress", - Name: "nginx-ingress", - } - - _, err = configurator.AddOrUpdateMergeableIngress(test.ingress) - if err != nil { - t.Fatal(err) - } - - gotTraceDataOnAdd, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } - - if !cmp.Equal(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnAdd.ServiceCount, gotTraceDataOnAdd.ServiceCount)) - } - - for i := 0; i < test.deleteCount; i++ { - ing := test.ingress + c.Collect(context.Background()) - key := fmt.Sprintf("%s/%s", ing.Master.Ingress.Namespace, ing.Master.Ingress.Name) - err := configurator.DeleteIngress(key, false) - if err != nil { - t.Fatal(err) + telData := tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ProjectArchitecture: telemetryNICData.ProjectArchitecture, + ClusterID: telemetryNICData.ClusterID, + ClusterVersion: telemetryNICData.ClusterVersion, } - } + clusterIPServices := tc.want["ClusterIP"] + nodePortServices := tc.want["NodePort"] + externalNameServices := tc.want["ExternalName"] - if err != nil { - t.Fatal(err) - } + nicResourceCounts := telemetry.NICResourceCounts{ + ClusterIPServices: int64(clusterIPServices), + NodePortServices: int64(nodePortServices), + ExternalNameServices: int64(externalNameServices), + } - gotTraceDataOnDelete, err := c.BuildReport(context.Background()) - if err != nil { - t.Fatal(err) - } + td := telemetry.Data{ + Data: telData, + NICResourceCounts: nicResourceCounts, + } - if !cmp.Equal(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount) { - t.Error(cmp.Diff(test.expectedTraceDataOnDelete.ServiceCount, gotTraceDataOnDelete.ServiceCount)) - } + want := fmt.Sprintf("%+v", &td) + got := buf.String() + if cmp.Equal(want, got) { + t.Error(cmp.Diff(want, got)) + } + }) } } diff --git a/internal/telemetry/data.avdl b/internal/telemetry/data.avdl index 4b96dfe9de..c51ba6e2e2 100644 --- a/internal/telemetry/data.avdl +++ b/internal/telemetry/data.avdl @@ -48,8 +48,17 @@ It is the UID of the `kube-system` Namespace. */ /** Secrets is the number of Secret resources managed by the Ingress Controller. */ long? Secrets = null; - /** Services is the number of services referenced by NGINX Ingress Controller in the cluster */ - long? Services = null; + /** ClusterIPServices is the number of ClusterIP services managed by NGINX Ingress Controller. */ + long? ClusterIPServices = null; + + /** NodePortServices is the number of NodePort services managed by NGINX Ingress Controller. */ + long? NodePortServices = null; + + /** LoadBalancerServices is the number of LoadBalancer services managed by NGINX Ingress Controller. */ + long? LoadBalancerServices = null; + + /** ExternalNameServices is the number of ExternalName services managed by NGINX Ingress Controller. */ + long? ExternalNameServices = null; /** RegularIngressCount is the number of Regular Ingress resources managed by NGINX Ingress Controller. */ long? RegularIngressCount = null; diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index 16067c527a..942940fdb8 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -75,8 +75,14 @@ type NICResourceCounts struct { Replicas int64 // Secrets is the number of Secret resources managed by the Ingress Controller. Secrets int64 - // Services is the number of services referenced by NGINX Ingress Controller in the cluster - Services int64 + // ClusterIPServices is the number of ClusterIP services managed by NGINX Ingress Controller. + ClusterIPServices int64 + // NodePortServices is the number of NodePort services managed by NGINX Ingress Controller. + NodePortServices int64 + // LoadBalancerServices is the number of LoadBalancer services managed by NGINX Ingress Controller. + LoadBalancerServices int64 + // ExternalNameServices is the number of ExternalName services managed by NGINX Ingress Controller. + ExternalNameServices int64 // RegularIngressCount is the number of Regular Ingress resources managed by NGINX Ingress Controller. RegularIngressCount int64 // MasterIngressCount is the number of Regular Ingress resources managed by NGINX Ingress Controller. diff --git a/internal/telemetry/nicresourcecounts_attributes_generated.go b/internal/telemetry/nicresourcecounts_attributes_generated.go index 36361cf574..21be8dcc3f 100644 --- a/internal/telemetry/nicresourcecounts_attributes_generated.go +++ b/internal/telemetry/nicresourcecounts_attributes_generated.go @@ -18,7 +18,10 @@ func (d *NICResourceCounts) Attributes() []attribute.KeyValue { attrs = append(attrs, attribute.Int64("TransportServers", d.TransportServers)) attrs = append(attrs, attribute.Int64("Replicas", d.Replicas)) attrs = append(attrs, attribute.Int64("Secrets", d.Secrets)) - attrs = append(attrs, attribute.Int64("Services", d.Services)) + attrs = append(attrs, attribute.Int64("ClusterIPServices", d.ClusterIPServices)) + attrs = append(attrs, attribute.Int64("NodePortServices", d.NodePortServices)) + attrs = append(attrs, attribute.Int64("LoadBalancerServices", d.LoadBalancerServices)) + attrs = append(attrs, attribute.Int64("ExternalNameServices", d.ExternalNameServices)) attrs = append(attrs, attribute.Int64("RegularIngressCount", d.RegularIngressCount)) attrs = append(attrs, attribute.Int64("MasterIngressCount", d.MasterIngressCount)) attrs = append(attrs, attribute.Int64("MinionIngressCount", d.MinionIngressCount))