From a7cbbbcab08ba17ad7a562792ea3fdc7e369affe Mon Sep 17 00:00:00 2001
From: Miles Garnsey <11435896+Miles-Garnsey@users.noreply.github.com>
Date: Thu, 9 Mar 2023 09:56:11 +1100
Subject: [PATCH] Default metrics filters for new metrics agent (#834)

* Start stubbing out methods for agent config.
* Add reconciliation logic, tests.
* CRD upgrades, codegen upgrades.
* Make type names more concise.
* The Endpoint type within the CassandraAgentSpec needs to be a pointer as it must be optional from the perspective of the CR so that it can have defaulting behaviour in the controller.
* Replace Cassandra agent `filters` field with `relabels`.
* Update metric-collector.yaml to metrics-collector.yaml
* Make changes to default metrics so that new metrics name format is correctly parsed by regex.
---
 .DS_Store                                     | Bin 6148 -> 0 bytes
 apis/telemetry/v1alpha1/telemetry_types.go    |   4 +-
 .../v1alpha1/zz_generated.deepcopy.go         |  10 +-
 .../bases/k8ssandra.io_k8ssandraclusters.yaml |  12 +-
 .../bases/reaper.k8ssandra.io_reapers.yaml    |   2 +-
 .../stargate.k8ssandra.io_stargates.yaml      |   4 +-
 .../en/components/metrics-collector/_index.md |   2 +-
 docs/content/en/tasks/monitor/_index.md       |   1 -
 .../cassandra_agent/cassandra_agent_config.go |  89 ++++++++++-
 .../cassandra_agent_config_test.go            | 138 +++++++++++++++++-
 10 files changed, 236 insertions(+), 26 deletions(-)

diff --git a/.DS_Store b/.DS_Store
index 5008ddfcf53c02e82d7eee2e57c38e5672ef89f6..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644
GIT binary patch
literal 0
HcmV?d00001

literal 6148
zcmeH~Jr2S!425mzP>H1@V-^m;4Wg<&0T*E43hX&L&p$$qDprKhvt+--jT7}7np#A3
zem<@ulZcFPQ@L2!n>{z**<q8>++&mCkOWA81W14cNZ<zv;LbK1Poaz?KmsK2CSc!(
z0ynLxE!0092;Krf2c+FF_Fe*7ECH>lEfg7;MkzE(HCqgga^y>{tEnwC%0;vJ&^%eQ
zLs35+`xjp>T0<F0fCPF1$Cyrb|F7^5{eNG?83~ZUUlGt@xh*qZDeu<Z%US-OSsOPv
j)R!Z4KLME7ReXlK;d!wEw5GODWMKRea10D2@KpjYNUI8I

diff --git a/apis/telemetry/v1alpha1/telemetry_types.go b/apis/telemetry/v1alpha1/telemetry_types.go
index a935ec012..ec85f84f8 100644
--- a/apis/telemetry/v1alpha1/telemetry_types.go
+++ b/apis/telemetry/v1alpha1/telemetry_types.go
@@ -130,8 +130,8 @@ type McacTelemetrySpec struct {
 }
 
 type CassandraAgentSpec struct {
-	Endpoint Endpoint                `json:"endpoint,omitempty"`
-	Filters  []promapi.RelabelConfig `json:"filters,omitempty"`
+	Endpoint *Endpoint               `json:"endpoint,omitempty"`
+	Relabels []promapi.RelabelConfig `json:"relabels,omitempty"`
 }
 
 type Endpoint struct {
diff --git a/apis/telemetry/v1alpha1/zz_generated.deepcopy.go b/apis/telemetry/v1alpha1/zz_generated.deepcopy.go
index 54da6841b..2a440e3c0 100644
--- a/apis/telemetry/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/telemetry/v1alpha1/zz_generated.deepcopy.go
@@ -30,9 +30,13 @@ import (
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *CassandraAgentSpec) DeepCopyInto(out *CassandraAgentSpec) {
 	*out = *in
-	out.Endpoint = in.Endpoint
-	if in.Filters != nil {
-		in, out := &in.Filters, &out.Filters
+	if in.Endpoint != nil {
+		in, out := &in.Endpoint, &out.Endpoint
+		*out = new(Endpoint)
+		**out = **in
+	}
+	if in.Relabels != nil {
+		in, out := &in.Relabels, &out.Relabels
 		*out = make([]monitoringv1.RelabelConfig, len(*in))
 		for i := range *in {
 			(*in)[i].DeepCopyInto(&(*out)[i])
diff --git a/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml b/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml
index eac3ea3a7..cda309b88 100644
--- a/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml
+++ b/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml
@@ -13740,7 +13740,7 @@ spec:
                                               port:
                                                 type: string
                                             type: object
-                                          filters:
+                                          relabels:
                                             items:
                                               description: 'RelabelConfig allows dynamic
                                                 rewriting of the label set, being
@@ -14259,7 +14259,7 @@ spec:
                                         port:
                                           type: string
                                       type: object
-                                    filters:
+                                    relabels:
                                       items:
                                         description: 'RelabelConfig allows dynamic
                                           rewriting of the label set, being applied
@@ -16841,7 +16841,7 @@ spec:
                                     port:
                                       type: string
                                   type: object
-                                filters:
+                                relabels:
                                   items:
                                     description: 'RelabelConfig allows dynamic rewriting
                                       of the label set, being applied to samples before
@@ -25518,7 +25518,7 @@ spec:
                               port:
                                 type: string
                             type: object
-                          filters:
+                          relabels:
                             items:
                               description: 'RelabelConfig allows dynamic rewriting
                                 of the label set, being applied to samples before
@@ -28195,7 +28195,7 @@ spec:
                               port:
                                 type: string
                             type: object
-                          filters:
+                          relabels:
                             items:
                               description: 'RelabelConfig allows dynamic rewriting
                                 of the label set, being applied to samples before
@@ -29864,7 +29864,7 @@ spec:
                               port:
                                 type: string
                             type: object
-                          filters:
+                          relabels:
                             items:
                               description: 'RelabelConfig allows dynamic rewriting
                                 of the label set, being applied to samples before
diff --git a/config/crd/bases/reaper.k8ssandra.io_reapers.yaml b/config/crd/bases/reaper.k8ssandra.io_reapers.yaml
index ffb8c9e5b..fcbe86856 100644
--- a/config/crd/bases/reaper.k8ssandra.io_reapers.yaml
+++ b/config/crd/bases/reaper.k8ssandra.io_reapers.yaml
@@ -2087,7 +2087,7 @@ spec:
                           port:
                             type: string
                         type: object
-                      filters:
+                      relabels:
                         items:
                           description: 'RelabelConfig allows dynamic rewriting of
                             the label set, being applied to samples before ingestion.
diff --git a/config/crd/bases/stargate.k8ssandra.io_stargates.yaml b/config/crd/bases/stargate.k8ssandra.io_stargates.yaml
index e630a422e..336d63e3f 100644
--- a/config/crd/bases/stargate.k8ssandra.io_stargates.yaml
+++ b/config/crd/bases/stargate.k8ssandra.io_stargates.yaml
@@ -2775,7 +2775,7 @@ spec:
                                 port:
                                   type: string
                               type: object
-                            filters:
+                            relabels:
                               items:
                                 description: 'RelabelConfig allows dynamic rewriting
                                   of the label set, being applied to samples before
@@ -3242,7 +3242,7 @@ spec:
                           port:
                             type: string
                         type: object
-                      filters:
+                      relabels:
                         items:
                           description: 'RelabelConfig allows dynamic rewriting of
                             the label set, being applied to samples before ingestion.
diff --git a/docs/content/en/components/metrics-collector/_index.md b/docs/content/en/components/metrics-collector/_index.md
index 7dcee955d..3fff56859 100644
--- a/docs/content/en/components/metrics-collector/_index.md
+++ b/docs/content/en/components/metrics-collector/_index.md
@@ -82,7 +82,7 @@ Ingress or port forwarding can be used to expose access to the Prometheus and Gr
 
   2. How can I filter out metrics I don't care about?
 
-     Please read the [metric-collector.yaml](https://github.com/datastax/metric-collector-for-apache-cassandra/blob/master/config/metric-collector.yaml) section in the MCAC GitHub repo on how to add filtering rules.
+     Please read the [metrics-collector.yaml](https://github.com/datastax/metric-collector-for-apache-cassandra/blob/master/config/metrics-collector.yaml) section in the MCAC GitHub repo on how to add filtering rules.
 
   3. What is the datalog? And what is it for?
 
diff --git a/docs/content/en/tasks/monitor/_index.md b/docs/content/en/tasks/monitor/_index.md
index 67c7fdead..1bde08f78 100644
--- a/docs/content/en/tasks/monitor/_index.md
+++ b/docs/content/en/tasks/monitor/_index.md
@@ -3,4 +3,3 @@ title: "Monitor K8ssandra"
 linkTitle: "Monitor"
 weight: 6
 description: "Access tools to monitor your Apache Cassandra® cluster running in Kubernetes."
----
\ No newline at end of file
diff --git a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go
index bf28daa96..a0311c750 100644
--- a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go
+++ b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go
@@ -5,12 +5,15 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/adutra/goalesce"
+
 	cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
 	k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1"
 	telemetryapi "github.com/k8ssandra/k8ssandra-operator/apis/telemetry/v1alpha1"
 	"github.com/k8ssandra/k8ssandra-operator/pkg/labels"
 	"github.com/k8ssandra/k8ssandra-operator/pkg/reconciliation"
 	"github.com/k8ssandra/k8ssandra-operator/pkg/result"
+	promapi "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -21,10 +24,89 @@ import (
 var (
 	agentConfigLocation = "/opt/management-api/configs/metrics-collector.yaml"
 	defaultAgentConfig  = telemetryapi.CassandraAgentSpec{
-		Endpoint: telemetryapi.Endpoint{
+		Endpoint: &telemetryapi.Endpoint{
 			Port:    "9000",
 			Address: "127.0.0.1",
 		},
+		Relabels: []promapi.RelabelConfig{
+			{
+				SourceLabels: []string{"table"},
+				Regex:        ".+",
+				TargetLabel:  "should_drop",
+				Replacement:  "true",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_live_ss_table_count",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_live_disk_space_used",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_memtable.*",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_all_memtables.*",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_compaction_bytes_written",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_pending_compactions",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_read_.*",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_write_.*",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_range.*",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_coordinator_.*",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"__name__"},
+				Regex:        "org_apache_cassandra_metrics_table_dropped_mutations",
+				TargetLabel:  "should_drop",
+				Replacement:  "false",
+			},
+			{
+				SourceLabels: []string{"should_drop"},
+				Regex:        "true",
+				Action:       "drop",
+			},
+		},
 	}
 )
 
@@ -42,7 +124,8 @@ func (c Configurator) GetTelemetryAgentConfigMap() (*corev1.ConfigMap, error) {
 	var yamlData []byte
 	var err error
 	if c.TelemetrySpec.Cassandra != nil {
-		yamlData, err = yaml.Marshal(&c.TelemetrySpec.Cassandra)
+		mergedSpec := goalesce.MustDeepMerge(&defaultAgentConfig, c.TelemetrySpec.Cassandra)
+		yamlData, err = yaml.Marshal(&mergedSpec)
 		if err != nil {
 			return &corev1.ConfigMap{}, err
 		}
@@ -79,7 +162,7 @@ func (c Configurator) ReconcileTelemetryAgentConfig(dc *cassdcapi.CassandraDatac
 	recRes := reconciliation.ReconcileObject(c.Ctx, c.RemoteClient, c.RequeueDelay, *desiredCm)
 	switch {
 	case recRes.IsError():
-		fallthrough
+		return recRes
 	case recRes.IsRequeue():
 		return recRes
 	}
diff --git a/pkg/telemetry/cassandra_agent/cassandra_agent_config_test.go b/pkg/telemetry/cassandra_agent/cassandra_agent_config_test.go
index 46328ee9c..a0149f6df 100644
--- a/pkg/telemetry/cassandra_agent/cassandra_agent_config_test.go
+++ b/pkg/telemetry/cassandra_agent/cassandra_agent_config_test.go
@@ -27,10 +27,90 @@ var (
 		DcNamespace:   testCluster.Spec.Cassandra.Datacenters[0].Meta.Namespace,
 		DcName:        testCluster.Spec.Cassandra.Datacenters[0].Meta.Name,
 	}
-	expectedYaml string = `endpoint:
+	allDefinedYaml string = `endpoint:
   address: 127.0.0.1
   port: "10000"
-filters:
+relabels:
+- action: drop
+  regex: (.*);(b.*)
+  separator: ;
+  sourceLabels:
+  - tag1
+  - tag2
+`
+	endpointDefinedYaml string = `endpoint:
+  address: 192.168.1.10
+  port: "50000"
+relabels:
+- regex: .+
+  replacement: "true"
+  sourceLabels:
+  - table
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_live_ss_table_count
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_live_disk_space_used
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_memtable.*
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_all_memtables.*
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_compaction_bytes_written
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_pending_compactions
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_read_.*
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_write_.*
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_range.*
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_coordinator_.*
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- regex: org_apache_cassandra_metrics_table_dropped_mutations
+  replacement: "false"
+  sourceLabels:
+  - __name__
+  targetLabel: should_drop
+- action: drop
+  regex: "true"
+  sourceLabels:
+  - should_drop
+`
+	relabelsDefinedYaml = `endpoint:
+  address: 127.0.0.1
+  port: "9000"
+relabels:
 - action: drop
   regex: (.*);(b.*)
   separator: ;
@@ -46,14 +126,14 @@ func getExpectedConfigMap() corev1.ConfigMap {
 			Namespace: Cfg.DcNamespace,
 			Name:      Cfg.Kluster.Name + "-" + Cfg.DcName + "-metrics-agent-config",
 		},
-		Data: map[string]string{filepath.Base(agentConfigLocation): expectedYaml},
+		Data: map[string]string{filepath.Base(agentConfigLocation): allDefinedYaml},
 	}
 	return expectedCm
 }
 
 func getExampleTelemetrySpec() telemetryapi.TelemetrySpec {
 	tspec := &Cfg.TelemetrySpec
-	tspec.Cassandra.Filters = []promapi.RelabelConfig{
+	tspec.Cassandra.Relabels = []promapi.RelabelConfig{
 		{
 			SourceLabels: []string{"tag1", "tag2"},
 			Separator:    ";",
@@ -61,16 +141,60 @@ func getExampleTelemetrySpec() telemetryapi.TelemetrySpec {
 			Action:       "drop",
 		},
 	}
-	tspec.Cassandra.Endpoint.Address = "127.0.0.1"
-	tspec.Cassandra.Endpoint.Port = "10000"
+	tspec.Cassandra.Endpoint = &telemetryapi.Endpoint{
+		Address: "127.0.0.1",
+		Port:    "10000",
+	}
 	return *tspec
 }
 
-func Test_GetTelemetryAgentConfigMap(t *testing.T) {
+// Make sure when both endpoint and relabels are defined they come through to yaml.
+func Test_GetTelemetryAgentConfigMapAllDefined(t *testing.T) {
+	expectedCm := getExpectedConfigMap()
+	Cfg.RemoteClient = testutils.NewFakeClientWRestMapper() // Reset the Client
+	Cfg.TelemetrySpec = getExampleTelemetrySpec()
+	cm, err := Cfg.GetTelemetryAgentConfigMap()
+	assert.NoError(t, err)
+	assert.Equal(t, expectedCm.Data["metrics-collector.yaml"], cm.Data["metrics-collector.yaml"])
+	assert.Equal(t, expectedCm.Name, cm.Name)
+	assert.Equal(t, expectedCm.Namespace, cm.Namespace)
+}
+
+// Make sure we get default relabels when only endpoint is defined in spec.
+func Test_GetTelemetryAgentConfigMapWithDefinedEndpoint(t *testing.T) {
+	expectedCm := getExpectedConfigMap()
+	expectedCm.Data[filepath.Base(agentConfigLocation)] = endpointDefinedYaml
+	Cfg.RemoteClient = testutils.NewFakeClientWRestMapper() // Reset the Client
+	Cfg.TelemetrySpec = getExampleTelemetrySpec()
+	Cfg.TelemetrySpec.Cassandra.Relabels = nil
+	Cfg.TelemetrySpec.Cassandra.Endpoint = &telemetryapi.Endpoint{
+		Address: "192.168.1.10",
+		Port:    "50000",
+	}
+	cm, err := Cfg.GetTelemetryAgentConfigMap()
+	println(cm.Data)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedCm.Data["metrics-collector.yaml"], cm.Data["metrics-collector.yaml"])
+	assert.Equal(t, expectedCm.Name, cm.Name)
+	assert.Equal(t, expectedCm.Namespace, cm.Namespace)
+}
+
+func Test_GetTelemetryAgentConfigMapWithDefinedRelabels(t *testing.T) {
 	expectedCm := getExpectedConfigMap()
+	expectedCm.Data[filepath.Base(agentConfigLocation)] = relabelsDefinedYaml
 	Cfg.RemoteClient = testutils.NewFakeClientWRestMapper() // Reset the Client
 	Cfg.TelemetrySpec = getExampleTelemetrySpec()
+	Cfg.TelemetrySpec.Cassandra.Relabels = []promapi.RelabelConfig{
+		{
+			SourceLabels: []string{"tag1", "tag2"},
+			Separator:    ";",
+			Regex:        "(.*);(b.*)",
+			Action:       "drop",
+		},
+	}
+	Cfg.TelemetrySpec.Cassandra.Endpoint = nil
 	cm, err := Cfg.GetTelemetryAgentConfigMap()
+	println(cm.Data)
 	assert.NoError(t, err)
 	assert.Equal(t, expectedCm.Data["metrics-collector.yaml"], cm.Data["metrics-collector.yaml"])
 	assert.Equal(t, expectedCm.Name, cm.Name)