Skip to content

Commit 13bc62d

Browse files
iblancasaiblancasa
authored
Fix labels for Service Monitors (#2878)
* Create a separate Service Monitor when the Prometheus exporter is present Signed-off-by: Israel Blancas <[email protected]> * Improve changelog Signed-off-by: Israel Blancas <[email protected]> * Fix prometheus-cr E2E test Signed-off-by: Israel Blancas <[email protected]> * Remove unused target Signed-off-by: Israel Blancas <[email protected]> * Add docstring Signed-off-by: Israel Blancas <[email protected]> * Fix typo Signed-off-by: Israel Blancas <[email protected]> * Change the label name Signed-off-by: Israel Blancas <[email protected]> * Change changelog description Signed-off-by: Israel Blancas <[email protected]> * Recover removed labels Signed-off-by: Israel Blancas <[email protected]> * Add missing labels Signed-off-by: Israel Blancas <[email protected]> * Remove wrong labels Signed-off-by: Israel Blancas <[email protected]> --------- Signed-off-by: Israel Blancas <[email protected]>
1 parent 9c1889d commit 13bc62d

22 files changed

+363
-110
lines changed

‎.chloggen/bug_2877.yaml

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: collector
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Create a Service Monitor for the monitoring service and another one for the collector service when the Prometheus exporter is used.
9+
10+
# One or more tracking issues related to the change
11+
issues: [2877]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext: |
17+
Create a Service Monitor for the collector Service when Prometheus exporter is used. A different Service Monitor is created for the monitoring service.
18+
This helps excluding the headless service (duplicating the metrics collection) and splits responsibilities between the two Service Monitors.
19+
Now, the operator.opentelemetry.io/collector-service-type label is used to differentiate the services.
20+
operator.opentelemetry.io/collector-monitoring-service and operator.opentelemetry.io/collector-headless-service are deprecated now.

‎controllers/builder_test.go

+29-18
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,13 @@ service:
259259
Name: "test-collector",
260260
Namespace: "test",
261261
Labels: map[string]string{
262-
"app.kubernetes.io/component": "opentelemetry-collector",
263-
"app.kubernetes.io/instance": "test.test",
264-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
265-
"app.kubernetes.io/name": "test-collector",
266-
"app.kubernetes.io/part-of": "opentelemetry",
267-
"app.kubernetes.io/version": "latest",
262+
"app.kubernetes.io/component": "opentelemetry-collector",
263+
"app.kubernetes.io/instance": "test.test",
264+
"app.kubernetes.io/managed-by": "opentelemetry-operator",
265+
"app.kubernetes.io/name": "test-collector",
266+
"app.kubernetes.io/part-of": "opentelemetry",
267+
"app.kubernetes.io/version": "latest",
268+
"operator.opentelemetry.io/collector-service-type": "base",
268269
},
269270
Annotations: nil,
270271
},
@@ -291,6 +292,7 @@ service:
291292
"app.kubernetes.io/part-of": "opentelemetry",
292293
"app.kubernetes.io/version": "latest",
293294
"operator.opentelemetry.io/collector-headless-service": "Exists",
295+
"operator.opentelemetry.io/collector-service-type": "headless",
294296
},
295297
Annotations: map[string]string{
296298
"service.beta.openshift.io/serving-cert-secret-name": "test-collector-headless-tls",
@@ -319,6 +321,7 @@ service:
319321
"app.kubernetes.io/name": "test-collector-monitoring",
320322
"app.kubernetes.io/part-of": "opentelemetry",
321323
"app.kubernetes.io/version": "latest",
324+
"operator.opentelemetry.io/collector-service-type": "monitoring",
322325
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
323326
},
324327
Annotations: nil,
@@ -506,12 +509,13 @@ service:
506509
Name: "test-collector",
507510
Namespace: "test",
508511
Labels: map[string]string{
509-
"app.kubernetes.io/component": "opentelemetry-collector",
510-
"app.kubernetes.io/instance": "test.test",
511-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
512-
"app.kubernetes.io/name": "test-collector",
513-
"app.kubernetes.io/part-of": "opentelemetry",
514-
"app.kubernetes.io/version": "latest",
512+
"app.kubernetes.io/component": "opentelemetry-collector",
513+
"app.kubernetes.io/instance": "test.test",
514+
"app.kubernetes.io/managed-by": "opentelemetry-operator",
515+
"app.kubernetes.io/name": "test-collector",
516+
"app.kubernetes.io/part-of": "opentelemetry",
517+
"app.kubernetes.io/version": "latest",
518+
"operator.opentelemetry.io/collector-service-type": "base",
515519
},
516520
Annotations: nil,
517521
},
@@ -537,6 +541,7 @@ service:
537541
"app.kubernetes.io/name": "test-collector",
538542
"app.kubernetes.io/part-of": "opentelemetry",
539543
"app.kubernetes.io/version": "latest",
544+
"operator.opentelemetry.io/collector-service-type": "headless",
540545
"operator.opentelemetry.io/collector-headless-service": "Exists",
541546
},
542547
Annotations: map[string]string{
@@ -566,6 +571,7 @@ service:
566571
"app.kubernetes.io/name": "test-collector-monitoring",
567572
"app.kubernetes.io/part-of": "opentelemetry",
568573
"app.kubernetes.io/version": "latest",
574+
"operator.opentelemetry.io/collector-service-type": "monitoring",
569575
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
570576
},
571577
Annotations: nil,
@@ -774,12 +780,13 @@ service:
774780
Name: "test-collector",
775781
Namespace: "test",
776782
Labels: map[string]string{
777-
"app.kubernetes.io/component": "opentelemetry-collector",
778-
"app.kubernetes.io/instance": "test.test",
779-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
780-
"app.kubernetes.io/name": "test-collector",
781-
"app.kubernetes.io/part-of": "opentelemetry",
782-
"app.kubernetes.io/version": "latest",
783+
"app.kubernetes.io/component": "opentelemetry-collector",
784+
"app.kubernetes.io/instance": "test.test",
785+
"app.kubernetes.io/managed-by": "opentelemetry-operator",
786+
"app.kubernetes.io/name": "test-collector",
787+
"app.kubernetes.io/part-of": "opentelemetry",
788+
"app.kubernetes.io/version": "latest",
789+
"operator.opentelemetry.io/collector-service-type": "base",
783790
},
784791
Annotations: nil,
785792
},
@@ -805,6 +812,7 @@ service:
805812
"app.kubernetes.io/name": "test-collector",
806813
"app.kubernetes.io/part-of": "opentelemetry",
807814
"app.kubernetes.io/version": "latest",
815+
"operator.opentelemetry.io/collector-service-type": "headless",
808816
"operator.opentelemetry.io/collector-headless-service": "Exists",
809817
},
810818
Annotations: map[string]string{
@@ -834,6 +842,7 @@ service:
834842
"app.kubernetes.io/name": "test-collector-monitoring",
835843
"app.kubernetes.io/part-of": "opentelemetry",
836844
"app.kubernetes.io/version": "latest",
845+
"operator.opentelemetry.io/collector-service-type": "monitoring",
837846
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
838847
},
839848
Annotations: nil,
@@ -1317,6 +1326,7 @@ service:
13171326
"app.kubernetes.io/name": "test-collector-monitoring",
13181327
"app.kubernetes.io/part-of": "opentelemetry",
13191328
"app.kubernetes.io/version": "latest",
1329+
"operator.opentelemetry.io/collector-service-type": "monitoring",
13201330
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
13211331
},
13221332
Annotations: nil,
@@ -1711,6 +1721,7 @@ prometheus_cr:
17111721
"app.kubernetes.io/name": "test-collector-monitoring",
17121722
"app.kubernetes.io/part-of": "opentelemetry",
17131723
"app.kubernetes.io/version": "latest",
1724+
"operator.opentelemetry.io/collector-service-type": "monitoring",
17141725
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
17151726
},
17161727
Annotations: nil,

‎internal/manifests/collector/collector.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func Build(params manifests.Params) ([]client.Object, error) {
5757
if params.OtelCol.Spec.Mode == v1beta1.ModeSidecar {
5858
manifestFactories = append(manifestFactories, manifests.Factory(PodMonitor))
5959
} else {
60-
manifestFactories = append(manifestFactories, manifests.Factory(ServiceMonitor))
60+
manifestFactories = append(manifestFactories, manifests.Factory(ServiceMonitor), manifests.Factory(ServiceMonitorMonitoring))
6161
}
6262
}
6363

‎internal/manifests/collector/podmonitor.go

+21-16
Original file line numberDiff line numberDiff line change
@@ -31,28 +31,14 @@ import (
3131

3232
// PodMonitor returns the pod monitor for the given instance.
3333
func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) {
34-
if !params.OtelCol.Spec.Observability.Metrics.EnableMetrics {
35-
params.Log.V(2).Info("Metrics disabled for this OTEL Collector",
36-
"params.OtelCol.name", params.OtelCol.Name,
37-
"params.OtelCol.namespace", params.OtelCol.Namespace,
38-
)
39-
return nil, nil
40-
} else if params.Config.PrometheusCRAvailability() == prometheus.NotAvailable {
41-
params.Log.V(1).Info("Cannot enable PodMonitor when prometheus CRDs are unavailable",
42-
"params.OtelCol.name", params.OtelCol.Name,
43-
"params.OtelCol.namespace", params.OtelCol.Namespace,
44-
)
34+
if !shouldCreatePodMonitor(params) {
4535
return nil, nil
4636
}
47-
var pm monitoringv1.PodMonitor
4837

49-
if params.OtelCol.Spec.Mode != v1beta1.ModeSidecar {
50-
return nil, nil
51-
}
5238
name := naming.PodMonitor(params.OtelCol.Name)
5339
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, nil)
5440
selectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector)
55-
pm = monitoringv1.PodMonitor{
41+
pm := monitoringv1.PodMonitor{
5642
ObjectMeta: metav1.ObjectMeta{
5743
Namespace: params.OtelCol.Namespace,
5844
Name: name,
@@ -107,3 +93,22 @@ func metricsEndpointsFromConfig(logger logr.Logger, otelcol v1beta1.OpenTelemetr
10793
}
10894
return metricsEndpoints
10995
}
96+
97+
func shouldCreatePodMonitor(params manifests.Params) bool {
98+
l := params.Log.WithValues(
99+
"params.OtelCol.name", params.OtelCol.Name,
100+
"params.OtelCol.namespace", params.OtelCol.Namespace,
101+
)
102+
103+
if !params.OtelCol.Spec.Observability.Metrics.EnableMetrics {
104+
l.V(2).Info("Metrics disabled for this OTEL Collector. PodMonitor will not ve created")
105+
return false
106+
} else if params.Config.PrometheusCRAvailability() == prometheus.NotAvailable {
107+
l.V(2).Info("Cannot enable PodMonitor when prometheus CRDs are unavailable")
108+
return false
109+
} else if params.OtelCol.Spec.Mode != v1beta1.ModeSidecar {
110+
l.V(2).Info("Not using sidecar mode. PodMonitor will not be created")
111+
return false
112+
}
113+
return true
114+
}

‎internal/manifests/collector/service.go

+20-4
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,26 @@ import (
2929
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
3030
)
3131

32-
// headless and monitoring labels are to differentiate the headless/monitoring services from the clusterIP service.
32+
// headless and monitoring labels are to differentiate the base/headless/monitoring services from the clusterIP service.
3333
const (
34-
headlessLabel = "operator.opentelemetry.io/collector-headless-service"
35-
monitoringLabel = "operator.opentelemetry.io/collector-monitoring-service"
36-
valueExists = "Exists"
34+
headlessLabel = "operator.opentelemetry.io/collector-headless-service"
35+
monitoringLabel = "operator.opentelemetry.io/collector-monitoring-service"
36+
serviceTypeLabel = "operator.opentelemetry.io/collector-service-type"
37+
valueExists = "Exists"
3738
)
3839

40+
type ServiceType int
41+
42+
const (
43+
BaseServiceType ServiceType = iota
44+
HeadlessServiceType
45+
MonitoringServiceType
46+
)
47+
48+
func (s ServiceType) String() string {
49+
return [...]string{"base", "headless", "monitoring"}[s]
50+
}
51+
3952
func HeadlessService(params manifests.Params) (*corev1.Service, error) {
4053
h, err := Service(params)
4154
if h == nil || err != nil {
@@ -44,6 +57,7 @@ func HeadlessService(params manifests.Params) (*corev1.Service, error) {
4457

4558
h.Name = naming.HeadlessService(params.OtelCol.Name)
4659
h.Labels[headlessLabel] = valueExists
60+
h.Labels[serviceTypeLabel] = HeadlessServiceType.String()
4761

4862
// copy to avoid modifying params.OtelCol.Annotations
4963
annotations := map[string]string{
@@ -63,6 +77,7 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) {
6377
name := naming.MonitoringService(params.OtelCol.Name)
6478
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
6579
labels[monitoringLabel] = valueExists
80+
labels[serviceTypeLabel] = MonitoringServiceType.String()
6681

6782
metricsPort, err := params.OtelCol.Spec.Config.Service.MetricsPort()
6883
if err != nil {
@@ -90,6 +105,7 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) {
90105
func Service(params manifests.Params) (*corev1.Service, error) {
91106
name := naming.Service(params.OtelCol.Name)
92107
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
108+
labels[serviceTypeLabel] = BaseServiceType.String()
93109

94110
out, err := params.OtelCol.Spec.Config.Yaml()
95111
if err != nil {

‎internal/manifests/collector/service_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ func service(name string, ports []v1beta1.PortsSpec) v1.Service {
286286
func serviceWithInternalTrafficPolicy(name string, ports []v1beta1.PortsSpec, internalTrafficPolicy v1.ServiceInternalTrafficPolicyType) v1.Service {
287287
params := deploymentParams()
288288
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
289+
labels[serviceTypeLabel] = BaseServiceType.String()
289290

290291
svcPorts := []v1.ServicePort{}
291292
for _, p := range ports {

‎internal/manifests/collector/servicemonitor.go

+48-22
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package collector
1616

1717
import (
18+
"fmt"
1819
"strings"
1920

2021
"github.com/go-logr/logr"
@@ -29,30 +30,40 @@ import (
2930
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
3031
)
3132

32-
// ServiceMonitor returns the service monitor for the given instance.
33+
// ServiceMonitor returns the service monitor for the collector.
3334
func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, error) {
34-
if !params.OtelCol.Spec.Observability.Metrics.EnableMetrics {
35-
params.Log.V(2).Info("Metrics disabled for this OTEL Collector",
36-
"params.OtelCol.name", params.OtelCol.Name,
37-
"params.OtelCol.namespace", params.OtelCol.Namespace,
38-
)
39-
return nil, nil
40-
} else if params.Config.PrometheusCRAvailability() == prometheus.NotAvailable {
41-
params.Log.V(1).Info("Cannot enable ServiceMonitor when prometheus CRDs are unavailable",
42-
"params.OtelCol.name", params.OtelCol.Name,
43-
"params.OtelCol.namespace", params.OtelCol.Namespace,
44-
)
45-
return nil, nil
35+
name := naming.ServiceMonitor(params.OtelCol.Name)
36+
endpoints := endpointsFromConfig(params.Log, params.OtelCol)
37+
if len(endpoints) > 0 {
38+
return createServiceMonitor(name, params, BaseServiceType, endpoints)
4639
}
47-
var sm monitoringv1.ServiceMonitor
40+
return nil, nil
41+
}
42+
43+
// ServiceMonitor returns the service monitor for the monitoring service of the collector.
44+
func ServiceMonitorMonitoring(params manifests.Params) (*monitoringv1.ServiceMonitor, error) {
45+
name := naming.ServiceMonitor(fmt.Sprintf("%s-monitoring", params.OtelCol.Name))
46+
endpoints := []monitoringv1.Endpoint{
47+
{
48+
Port: "monitoring",
49+
},
50+
}
51+
return createServiceMonitor(name, params, MonitoringServiceType, endpoints)
52+
}
4853

49-
if params.OtelCol.Spec.Mode == v1beta1.ModeSidecar {
54+
// createServiceMonitor creates a Service Monitor using the provided name, the params from the instance, a label to identify the service
55+
// to target (like the monitoring or the collector services) and the endpoints to scrape.
56+
func createServiceMonitor(name string, params manifests.Params, serviceType ServiceType, endpoints []monitoringv1.Endpoint) (*monitoringv1.ServiceMonitor, error) {
57+
if !shouldCreateServiceMonitor(params) {
5058
return nil, nil
5159
}
52-
name := naming.ServiceMonitor(params.OtelCol.Name)
60+
61+
var sm monitoringv1.ServiceMonitor
62+
5363
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
5464
selectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector)
55-
selectorLabels[monitoringLabel] = valueExists
65+
// This label is the one which differentiates the services
66+
selectorLabels[serviceTypeLabel] = serviceType.String()
5667

5768
sm = monitoringv1.ServiceMonitor{
5869
ObjectMeta: metav1.ObjectMeta{
@@ -61,11 +72,7 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
6172
Labels: labels,
6273
},
6374
Spec: monitoringv1.ServiceMonitorSpec{
64-
Endpoints: append([]monitoringv1.Endpoint{
65-
{
66-
Port: "monitoring",
67-
},
68-
}, endpointsFromConfig(params.Log, params.OtelCol)...),
75+
Endpoints: endpoints,
6976
NamespaceSelector: monitoringv1.NamespaceSelector{
7077
MatchNames: []string{params.OtelCol.Namespace},
7178
},
@@ -78,6 +85,25 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
7885
return &sm, nil
7986
}
8087

88+
func shouldCreateServiceMonitor(params manifests.Params) bool {
89+
l := params.Log.WithValues(
90+
"params.OtelCol.name", params.OtelCol.Name,
91+
"params.OtelCol.namespace", params.OtelCol.Namespace,
92+
)
93+
94+
if !params.OtelCol.Spec.Observability.Metrics.EnableMetrics {
95+
l.V(2).Info("Metrics disabled for this OTEL Collector. ServiceMonitor will not ve created")
96+
return false
97+
} else if params.Config.PrometheusCRAvailability() == prometheus.NotAvailable {
98+
l.V(2).Info("Cannot enable ServiceMonitor when prometheus CRDs are unavailable")
99+
return false
100+
} else if params.OtelCol.Spec.Mode == v1beta1.ModeSidecar {
101+
l.V(2).Info("Using sidecar mode. ServiceMonitor will not be created")
102+
return false
103+
}
104+
return true
105+
}
106+
81107
func endpointsFromConfig(logger logr.Logger, otelcol v1beta1.OpenTelemetryCollector) []monitoringv1.Endpoint {
82108
// TODO: https://github.com/open-telemetry/opentelemetry-operator/issues/2603
83109
cfgStr, err := otelcol.Spec.Config.Yaml()

0 commit comments

Comments
 (0)