Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing label servicemonitor #2573

Merged
merged 15 commits into from
Mar 11, 2024
Merged

Add missing label servicemonitor #2573

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ea1e00a
Add missing label for Service/Pod Monitors
yuriolisa Jan 29, 2024
3f6fd0b
Added manifests.utils to set labels
yuriolisa Jan 30, 2024
32bf984
Fixed labels names
yuriolisa Jan 31, 2024
b456ba6
Fixed labels for PodMonitor
yuriolisa Feb 12, 2024
fc03579
Fixed e2e test
yuriolisa Feb 12, 2024
00d2a04
Fixed e2e test
yuriolisa Feb 13, 2024
2ee13bb
Fixed selectorMatchLabels
yuriolisa Feb 13, 2024
81ae4ab
Removed the extra method
yuriolisa Feb 29, 2024
a8a518a
Adjusted nil
yuriolisa Mar 1, 2024
525eb94
Merge branch 'main' into add-missing-label-servicemonitor
yuriolisa Mar 1, 2024
fd39eb9
Changed labels
yuriolisa Mar 7, 2024
50a375d
Changed labels
yuriolisa Mar 7, 2024
956b94e
Readded common labels
yuriolisa Mar 7, 2024
d41662b
Readded common labels
yuriolisa Mar 7, 2024
38464fb
Added labels assert
yuriolisa Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .chloggen/add-missing-label-servicemonitor.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
component: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Added missing label for Service/Pod Monitors

# One or more tracking issues related to the change
issues: [2251]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
21 changes: 8 additions & 13 deletions internal/manifests/collector/podmonitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package collector

import (
"fmt"
"strings"

"github.com/go-logr/logr"
Expand All @@ -25,10 +24,11 @@ import (
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

// ServiceMonitor returns the service monitor for the given instance.
// PodMonitor returns the pod monitor for the given instance.
func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) {
if !params.OtelCol.Spec.Observability.Metrics.EnableMetrics {
params.Log.V(2).Info("Metrics disabled for this OTEL Collector",
Expand All @@ -42,16 +42,14 @@ func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) {
if params.OtelCol.Spec.Mode != v1alpha1.ModeSidecar {
return nil, nil
}

name := naming.PodMonitor(params.OtelCol.Name)
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
yuriolisa marked this conversation as resolved.
Show resolved Hide resolved
selectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector)
pm = monitoringv1.PodMonitor{
ObjectMeta: metav1.ObjectMeta{
Namespace: params.OtelCol.Namespace,
Name: naming.PodMonitor(params.OtelCol.Name),
Labels: map[string]string{
"app.kubernetes.io/name": naming.PodMonitor(params.OtelCol.Name),
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
},
Name: name,
Labels: labels,
},
Spec: monitoringv1.PodMonitorSpec{
JobLabel: "app.kubernetes.io/instance",
Expand All @@ -60,10 +58,7 @@ func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) {
MatchNames: []string{params.OtelCol.Namespace},
},
Selector: metav1.LabelSelector{
MatchLabels: map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
},
MatchLabels: selectorLabels,
},
PodMetricsEndpoints: append(
[]monitoringv1.PodMetricsEndpoint{
Expand Down
20 changes: 18 additions & 2 deletions internal/manifests/collector/podmonitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,33 @@ func TestDesiredPodMonitors(t *testing.T) {
assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
assert.Equal(t, "monitoring", actual.Spec.PodMetricsEndpoints[0].Port)
expectedSelectorLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
}
assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
}

params, err = newParams("", "testdata/prometheus-exporter.yaml")
func TestDesiredPodMonitorsWithPrometheus(t *testing.T) {
params, err := newParams("", "testdata/prometheus-exporter.yaml")
assert.NoError(t, err)
params.OtelCol.Spec.Mode = v1alpha1.ModeSidecar
params.OtelCol.Spec.Observability.Metrics.EnableMetrics = true
actual, err = PodMonitor(params)
actual, err := PodMonitor(params)
assert.NoError(t, err)
assert.NotNil(t, actual)
assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
assert.Equal(t, "monitoring", actual.Spec.PodMetricsEndpoints[0].Port)
assert.Equal(t, "prometheus-dev", actual.Spec.PodMetricsEndpoints[1].Port)
assert.Equal(t, "prometheus-prod", actual.Spec.PodMetricsEndpoints[2].Port)
expectedSelectorLabels := map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/component": "opentelemetry-collector",
}
assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
}
18 changes: 7 additions & 11 deletions internal/manifests/collector/servicemonitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package collector

import (
"fmt"
"strings"

"github.com/go-logr/logr"
Expand All @@ -25,6 +24,7 @@ import (
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

Expand All @@ -42,15 +42,14 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
if params.OtelCol.Spec.Mode == v1alpha1.ModeSidecar {
return nil, nil
}
name := naming.ServiceMonitor(params.OtelCol.Name)
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
selectorMatchLabels := manifestutils.SelectorMatchLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector)
sm = monitoringv1.ServiceMonitor{
ObjectMeta: metav1.ObjectMeta{
Namespace: params.OtelCol.Namespace,
Name: naming.ServiceMonitor(params.OtelCol.Name),
Labels: map[string]string{
"app.kubernetes.io/name": naming.ServiceMonitor(params.OtelCol.Name),
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
},
Name: name,
Labels: labels,
},
Spec: monitoringv1.ServiceMonitorSpec{
Endpoints: append([]monitoringv1.Endpoint{
Expand All @@ -62,10 +61,7 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
MatchNames: []string{params.OtelCol.Namespace},
},
Selector: metav1.LabelSelector{
MatchLabels: map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
},
MatchLabels: selectorMatchLabels,
},
},
}
Expand Down
24 changes: 22 additions & 2 deletions internal/manifests/collector/servicemonitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"fmt"
"testing"

"github.com/open-telemetry/opentelemetry-operator/internal/naming"

"github.com/stretchr/testify/assert"
)

Expand All @@ -35,16 +37,34 @@ func TestDesiredServiceMonitors(t *testing.T) {
assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
assert.Equal(t, "monitoring", actual.Spec.Endpoints[0].Port)
expectedSelectorLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/name": naming.MonitoringService(params.OtelCol.Name),
}
assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
}

params, err = newParams("", "testdata/prometheus-exporter.yaml")
func TestDesiredServiceMonitorsWithPrometheus(t *testing.T) {
params, err := newParams("", "testdata/prometheus-exporter.yaml")
assert.NoError(t, err)
params.OtelCol.Spec.Observability.Metrics.EnableMetrics = true
actual, err = ServiceMonitor(params)
actual, err := ServiceMonitor(params)
assert.NoError(t, err)
assert.NotNil(t, actual)
assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
assert.Equal(t, "monitoring", actual.Spec.Endpoints[0].Port)
assert.Equal(t, "prometheus-dev", actual.Spec.Endpoints[1].Port)
assert.Equal(t, "prometheus-prod", actual.Spec.Endpoints[2].Port)
expectedSelectorLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/name": naming.MonitoringService(params.OtelCol.Name),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuriolisa i realized that we actually need to update the monitoring service for the collector to have its own special label in the same way we do for headless and instead match on that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is because the name label for the service actually is just the name of the overall collector

}
assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
}
11 changes: 11 additions & 0 deletions internal/manifests/manifestutils/labels.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,14 @@ func SelectorLabels(instance metav1.ObjectMeta, component string) map[string]str
"app.kubernetes.io/component": component,
}
}

// SelectorMatchLabels return the common labels that should be used on ServiceMonitor when the Spec.Observability.Metrics.EnableMetrics is true.
func SelectorMatchLabels(instance metav1.ObjectMeta, component string) map[string]string {
jaronoff97 marked this conversation as resolved.
Show resolved Hide resolved
return map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": naming.Truncate("%s.%s", 63, instance.Namespace, instance.Name),
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/component": component,
"app.kubernetes.io/name": naming.MonitoringService(instance.Name),
}
}
20 changes: 20 additions & 0 deletions internal/manifests/manifestutils/labels_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,23 @@ func TestSelectorLabels(t *testing.T) {
// verify
assert.Equal(t, expected, result)
}

func TestSelectorMatchLabels(t *testing.T) {
// prepare
expected := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-opentelemetry",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/name": "my-opentelemetry-collector-monitoring",
}
otelcol := v1alpha1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{Name: "my-opentelemetry", Namespace: "my-namespace"},
}

// test
result := SelectorMatchLabels(otelcol.ObjectMeta, "opentelemetry-collector")

// verify
assert.Equal(t, expected, result)
}
1 change: 1 addition & 0 deletions tests/e2e-prometheuscr/create-pm-prometheus-exporters/01-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ spec:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-pm-prometheus.simplest

1 change: 1 addition & 0 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/01-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ spec:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
---
apiVersion: v1
kind: Service
Expand Down
1 change: 1 addition & 0 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/02-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ spec:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
---
apiVersion: v1
kind: Service
Expand Down
1 change: 1 addition & 0 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/05-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ spec:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
1 change: 1 addition & 0 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/06-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ spec:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
jaronoff97 marked this conversation as resolved.
Show resolved Hide resolved
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
Loading