Skip to content

Commit e107ffe

Browse files
rashmichandrashekarjaronoff97
rashmichandrashekar
and
jaronoff97
authored
Fix for unmarshalling error for keepequal/dropequal (#2794)
* fix * add require * fixing lint errors * adding unit tests * fixing some yaml * adding few more tests * fixing lint * adding comment * Update cmd/otel-allocator/server/server.go Co-authored-by: Jacob Aronoff <[email protected]> --------- Co-authored-by: Jacob Aronoff <[email protected]>
1 parent a3919b9 commit e107ffe

File tree

6 files changed

+343
-1
lines changed

6 files changed

+343
-1
lines changed

‎.chloggen/fix-keepequal-dropequal.yaml

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
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. operator, target allocator, github action)
5+
component: target allocator
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Fix for keepequal/dropequal action
9+
10+
# One or more tracking issues related to the change
11+
issues: [2793]
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:

‎cmd/otel-allocator/server/server.go

+52-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package server
1616

1717
import (
1818
"context"
19+
"encoding/json"
1920
"fmt"
2021
"net/http"
2122
"net/http/pprof"
@@ -106,6 +107,50 @@ func (s *Server) Shutdown(ctx context.Context) error {
106107
return s.server.Shutdown(ctx)
107108
}
108109

110+
// RemoveRegexFromRelabelAction is needed specifically for keepequal/dropequal actions because even though the user doesn't specify the
111+
// regex field for these actions the unmarshalling implementations of prometheus adds back the default regex fields
112+
// which in turn causes the receiver to error out since the unmarshaling of the json response doesn't expect anything in the regex fields
113+
// for these actions. Adding this as a fix until the original issue with prometheus unmarshaling is fixed -
114+
// https://github.com/prometheus/prometheus/issues/12534
115+
func RemoveRegexFromRelabelAction(jsonConfig []byte) ([]byte, error) {
116+
var jobToScrapeConfig map[string]interface{}
117+
err := json.Unmarshal(jsonConfig, &jobToScrapeConfig)
118+
if err != nil {
119+
return nil, err
120+
}
121+
for _, scrapeConfig := range jobToScrapeConfig {
122+
scrapeConfig := scrapeConfig.(map[string]interface{})
123+
if scrapeConfig["relabel_configs"] != nil {
124+
relabelConfigs := scrapeConfig["relabel_configs"].([]interface{})
125+
for _, relabelConfig := range relabelConfigs {
126+
relabelConfig := relabelConfig.(map[string]interface{})
127+
// Dropping regex key from the map since unmarshalling this on the client(metrics_receiver.go) results in error
128+
// because of the bug here - https://github.com/prometheus/prometheus/issues/12534
129+
if relabelConfig["action"] == "keepequal" || relabelConfig["action"] == "dropequal" {
130+
delete(relabelConfig, "regex")
131+
}
132+
}
133+
}
134+
if scrapeConfig["metric_relabel_configs"] != nil {
135+
metricRelabelConfigs := scrapeConfig["metric_relabel_configs"].([]interface{})
136+
for _, metricRelabelConfig := range metricRelabelConfigs {
137+
metricRelabelConfig := metricRelabelConfig.(map[string]interface{})
138+
// Dropping regex key from the map since unmarshalling this on the client(metrics_receiver.go) results in error
139+
// because of the bug here - https://github.com/prometheus/prometheus/issues/12534
140+
if metricRelabelConfig["action"] == "keepequal" || metricRelabelConfig["action"] == "dropequal" {
141+
delete(metricRelabelConfig, "regex")
142+
}
143+
}
144+
}
145+
}
146+
147+
jsonConfigNew, err := json.Marshal(jobToScrapeConfig)
148+
if err != nil {
149+
return nil, err
150+
}
151+
return jsonConfigNew, nil
152+
}
153+
109154
// UpdateScrapeConfigResponse updates the scrape config response. The target allocator first marshals these
110155
// configurations such that the underlying prometheus marshaling is used. After that, the YAML is converted
111156
// in to a JSON format for consumers to use.
@@ -120,8 +165,14 @@ func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.Scrap
120165
if err != nil {
121166
return err
122167
}
168+
169+
jsonConfigNew, err := RemoveRegexFromRelabelAction(jsonConfig)
170+
if err != nil {
171+
return err
172+
}
173+
123174
s.mtx.Lock()
124-
s.scrapeConfigResponse = jsonConfig
175+
s.scrapeConfigResponse = jsonConfigNew
125176
s.mtx.Unlock()
126177
return nil
127178
}

‎cmd/otel-allocator/server/server_test.go

+58
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
logf "sigs.k8s.io/controller-runtime/pkg/log"
3535

3636
"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation"
37+
allocatorconfig "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config"
3738
"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target"
3839
)
3940

@@ -612,6 +613,63 @@ func TestServer_Readiness(t *testing.T) {
612613
}
613614
}
614615

616+
func TestServer_ScrapeConfigRespose(t *testing.T) {
617+
tests := []struct {
618+
description string
619+
filePath string
620+
expectedCode int
621+
}{
622+
{
623+
description: "Jobs with all actions",
624+
filePath: "./testdata/prom-config-all-actions.yaml",
625+
expectedCode: http.StatusOK,
626+
},
627+
{
628+
description: "Jobs with config combinations",
629+
filePath: "./testdata/prom-config-test.yaml",
630+
expectedCode: http.StatusOK,
631+
},
632+
{
633+
description: "Jobs with no config",
634+
filePath: "./testdata/prom-no-config.yaml",
635+
expectedCode: http.StatusOK,
636+
},
637+
}
638+
for _, tc := range tests {
639+
t.Run(tc.description, func(t *testing.T) {
640+
listenAddr := ":8080"
641+
s := NewServer(logger, nil, listenAddr)
642+
643+
allocCfg := allocatorconfig.CreateDefaultConfig()
644+
err := allocatorconfig.LoadFromFile(tc.filePath, &allocCfg)
645+
require.NoError(t, err)
646+
647+
jobToScrapeConfig := make(map[string]*promconfig.ScrapeConfig)
648+
649+
for _, scrapeConfig := range allocCfg.PromConfig.ScrapeConfigs {
650+
jobToScrapeConfig[scrapeConfig.JobName] = scrapeConfig
651+
}
652+
653+
assert.NoError(t, s.UpdateScrapeConfigResponse(jobToScrapeConfig))
654+
655+
request := httptest.NewRequest("GET", "/scrape_configs", nil)
656+
w := httptest.NewRecorder()
657+
658+
s.server.Handler.ServeHTTP(w, request)
659+
result := w.Result()
660+
661+
assert.Equal(t, tc.expectedCode, result.StatusCode)
662+
bodyBytes, err := io.ReadAll(result.Body)
663+
require.NoError(t, err)
664+
665+
// Checking to make sure yaml unmarshaling doesn't result in errors for responses containing any supported prometheus relabel action
666+
scrapeConfigs := map[string]*promconfig.ScrapeConfig{}
667+
err = yaml.Unmarshal(bodyBytes, scrapeConfigs)
668+
require.NoError(t, err)
669+
})
670+
}
671+
}
672+
615673
func newLink(jobName string) target.LinkJSON {
616674
return target.LinkJSON{Link: fmt.Sprintf("/jobs/%s/targets", url.QueryEscape(jobName))}
617675
}

‎cmd/otel-allocator/server/testdata/prom-config-all-actions.yaml

+175
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
collector_selector:
2+
matchlabels:
3+
app.kubernetes.io/instance: default.test
4+
app.kubernetes.io/managed-by: opentelemetry-operator
5+
prometheus_cr:
6+
scrape_interval: 60s
7+
config:
8+
scrape_configs:
9+
- job_name: job-replace
10+
scheme: http
11+
kubernetes_sd_configs:
12+
- role: pod
13+
relabel_configs:
14+
- source_labels: [__meta_kubernetes_pod_label_app]
15+
action: keep
16+
regex: "prometheus-reference-app"
17+
- source_labels: [__meta_kubernetes_pod_label_test]
18+
action: replace
19+
regex: "test$1replacement"
20+
replacement: "myreplacement$$1"
21+
target_label: "mytarget"
22+
metric_relabel_configs:
23+
- action: replace
24+
source_labels: [city]
25+
separator: ","
26+
regex: (ci.*)(name$)
27+
replacement: "test_newest_1_$1"
28+
target_label: city
29+
- job_name: job-lowercase
30+
scheme: http
31+
kubernetes_sd_configs:
32+
- role: pod
33+
relabel_configs:
34+
- source_labels: [__meta_kubernetes_pod_label_app]
35+
action: keep
36+
regex: "prometheus-reference-app"
37+
- source_labels: [__meta_kubernetes_pod_label_test]
38+
action: lowercase
39+
target_label: "mytarget"
40+
metric_relabel_configs:
41+
- action: lowercase
42+
source_labels: [city]
43+
target_label: city
44+
- job_name: job-uppercase
45+
scheme: http
46+
kubernetes_sd_configs:
47+
- role: pod
48+
relabel_configs:
49+
- source_labels: [__meta_kubernetes_pod_label_app]
50+
action: keep
51+
regex: "prometheus-reference-app"
52+
- source_labels: [__meta_kubernetes_pod_label_test]
53+
action: uppercase
54+
target_label: "mytarget"
55+
metric_relabel_configs:
56+
- action: uppercase
57+
source_labels: [city]
58+
target_label: city
59+
- job_name: job-keep
60+
scheme: http
61+
kubernetes_sd_configs:
62+
- role: pod
63+
relabel_configs:
64+
- source_labels: [__meta_kubernetes_pod_label_app]
65+
action: keep
66+
regex: "prometheus-reference-app"
67+
metric_relabel_configs:
68+
- action: keep
69+
source_labels: [city]
70+
separator: ","
71+
regex: (ci.*)(name$)
72+
- job_name: job-drop
73+
scheme: http
74+
kubernetes_sd_configs:
75+
- role: pod
76+
relabel_configs:
77+
- source_labels: [__meta_kubernetes_pod_label_app]
78+
action: keep
79+
regex: "prometheus-reference-app"
80+
- source_labels: [__meta_kubernetes_pod_label_droplabel]
81+
action: drop
82+
regex: "prometheus-reference-app"
83+
metric_relabel_configs:
84+
- action: drop
85+
source_labels: [city]
86+
regex: (ci.*)(name$)
87+
- job_name: job-keepequal
88+
scheme: http
89+
kubernetes_sd_configs:
90+
- role: pod
91+
relabel_configs:
92+
- source_labels: [__meta_kubernetes_pod_label_app]
93+
action: keepequal
94+
target_label: __meta_kubernetes_pod_label_mylabel
95+
metric_relabel_configs:
96+
- action: keepequal
97+
source_labels: [city]
98+
target_label: city
99+
- job_name: job-dropequal
100+
scheme: http
101+
kubernetes_sd_configs:
102+
- role: pod
103+
relabel_configs:
104+
- source_labels: [__meta_kubernetes_pod_label_app]
105+
action: keep
106+
regex: "prometheus-reference-app"
107+
- source_labels: [__meta_kubernetes_pod_label_dropequallabel]
108+
action: dropequal
109+
target_label: "test"
110+
metric_relabel_configs:
111+
- action: dropequal
112+
source_labels: [citytest]
113+
target_label: city
114+
- job_name: job-hashmod
115+
scheme: http
116+
kubernetes_sd_configs:
117+
- role: pod
118+
relabel_configs:
119+
- source_labels: [__meta_kubernetes_pod_label_app]
120+
action: keep
121+
regex: "prometheus-reference-app"
122+
- source_labels: [__meta_kubernetes_pod_label_hashmod]
123+
action: hashmod
124+
modulus: 5
125+
regex: "prometheus-reference-app"
126+
target_label: city
127+
metric_relabel_configs:
128+
- action: hashmod
129+
modulus: 5
130+
source_labels: [city]
131+
regex: (ci.*)(name$)
132+
target_label: city
133+
- job_name: job-labelmap
134+
scheme: http
135+
kubernetes_sd_configs:
136+
- role: pod
137+
relabel_configs:
138+
- source_labels: [__meta_kubernetes_pod_label_app]
139+
action: keep
140+
regex: "prometheus-reference-app"
141+
- source_labels: [__meta_kubernetes_pod_label_hashmod]
142+
action: labelmap
143+
regex: "prometheus-reference-app"
144+
target_label: city
145+
metric_relabel_configs:
146+
- action: labelmap
147+
source_labels: [city]
148+
regex: (ci.*)(name$)
149+
target_label: city
150+
- job_name: job-labeldrop
151+
scheme: http
152+
kubernetes_sd_configs:
153+
- role: pod
154+
relabel_configs:
155+
- source_labels: [__meta_kubernetes_pod_label_app]
156+
action: keep
157+
regex: "prometheus-reference-app"
158+
- action: labeldrop
159+
regex: "prometheus-reference-app"
160+
metric_relabel_configs:
161+
- action: labeldrop
162+
regex: (ci.*)(name$)
163+
- job_name: job-labelkeep
164+
scheme: http
165+
kubernetes_sd_configs:
166+
- role: pod
167+
relabel_configs:
168+
- source_labels: [__meta_kubernetes_pod_label_app]
169+
action: keep
170+
regex: "prometheus-reference-app"
171+
- action: labelkeep
172+
regex: "prometheus-reference-app"
173+
metric_relabel_configs:
174+
- action: labelkeep
175+
regex: (ci.*)(name$$)

‎cmd/otel-allocator/server/testdata/prom-config-test.yaml

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
collector_selector:
2+
matchlabels:
3+
app.kubernetes.io/instance: default.test
4+
app.kubernetes.io/managed-by: opentelemetry-operator
5+
prometheus_cr:
6+
scrape_interval: 60s
7+
config:
8+
scrape_configs:
9+
- job_name: job-no-target-relabel
10+
scheme: http
11+
kubernetes_sd_configs:
12+
- role: pod
13+
metric_relabel_configs:
14+
- action: replace
15+
source_labels: [city]
16+
separator: ","
17+
regex: (ci.*)(name$)
18+
replacement: "test_newest_1_$1"
19+
target_label: city
20+
- job_name: job-no-metric-relabel
21+
scheme: http
22+
kubernetes_sd_configs:
23+
- role: pod
24+
relabel_configs:
25+
- source_labels: [__meta_kubernetes_pod_label_app]
26+
action: keep
27+
regex: "prometheus-reference-app"
28+
- source_labels: [__meta_kubernetes_pod_label_test]
29+
action: lowercase
30+
target_label: "mytarget"
31+
- job_name: job-no-relabel
32+
scheme: http
33+
kubernetes_sd_configs:
34+
- role: pod

‎cmd/otel-allocator/server/testdata/prom-no-config.yaml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
collector_selector:
2+
matchlabels:
3+
app.kubernetes.io/instance: default.test
4+
app.kubernetes.io/managed-by: opentelemetry-operator
5+
prometheus_cr:
6+
scrape_interval: 60s
7+
config:
8+
scrape_configs:

0 commit comments

Comments
 (0)