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.

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

Fix for unmarshalling error for keepequal/dropequal #2794

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3542e28
fix
rashmichandrashekar Mar 27, 2024
bbc908f
Merge branch 'main' into rashmi/keepequal-fix
rashmichandrashekar Apr 2, 2024
76cceab
add require
rashmichandrashekar Apr 2, 2024
359bd30
Merge branch 'rashmi/keepequal-fix' of https://github.com/rashmichand…
rashmichandrashekar Apr 2, 2024
ee10c4c
Merge branch 'main' into rashmi/keepequal-fix
rashmichandrashekar Apr 2, 2024
68514e3
fixing lint errors
rashmichandrashekar Apr 2, 2024
ded0d91
Merge branch 'rashmi/keepequal-fix' of https://github.com/rashmichand…
rashmichandrashekar Apr 2, 2024
028f90a
adding unit tests
rashmichandrashekar Apr 8, 2024
36e1a25
fixing some yaml
rashmichandrashekar Apr 8, 2024
e9ae63b
Merge branch 'main' into rashmi/keepequal-fix
rashmichandrashekar Apr 8, 2024
91d2ebe
adding few more tests
rashmichandrashekar Apr 9, 2024
b72563b
Merge branch 'rashmi/keepequal-fix' of https://github.com/rashmichand…
rashmichandrashekar Apr 9, 2024
e3ef0f9
Merge branch 'main' into rashmi/keepequal-fix
rashmichandrashekar Apr 9, 2024
af8e992
fixing lint
rashmichandrashekar Apr 9, 2024
9af2992
Merge branch 'main' into rashmi/keepequal-fix
rashmichandrashekar Apr 9, 2024
b9980e6
adding comment
rashmichandrashekar Apr 10, 2024
ceb1bd2
Merge branch 'rashmi/keepequal-fix' of https://github.com/rashmichand…
rashmichandrashekar Apr 10, 2024
bf11879
Update cmd/otel-allocator/server/server.go
rashmichandrashekar Apr 10, 2024
b8b4023
Merge branch 'main' into rashmi/keepequal-fix
rashmichandrashekar Apr 10, 2024
5b4ff61
Merge branch 'main' into rashmi/keepequal-fix
rashmichandrashekar Apr 11, 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/fix-keepequal-dropequal.yaml
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: target allocator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix for keepequal/dropequal action

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

# (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:
53 changes: 52 additions & 1 deletion cmd/otel-allocator/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package server

import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/pprof"
Expand Down Expand Up @@ -106,6 +107,50 @@ func (s *Server) Shutdown(ctx context.Context) error {
return s.server.Shutdown(ctx)
}

// RemoveRegexFromRelabelAction is needed specifically for keepequal/dropequal actions because even though the user doesn't specify the
// regex field for these actions the unmarshalling implementations of prometheus adds back the default regex fields
// which in turn causes the receiver to error out since the unmarshaling of the json response doesn't expect anything in the regex fields
// for these actions. Adding this as a fix until the original issue with prometheus unmarshaling is fixed -
// https://github.com/prometheus/prometheus/issues/12534
func RemoveRegexFromRelabelAction(jsonConfig []byte) ([]byte, error) {
var jobToScrapeConfig map[string]interface{}
err := json.Unmarshal(jsonConfig, &jobToScrapeConfig)
if err != nil {
return nil, err
}
for _, scrapeConfig := range jobToScrapeConfig {
scrapeConfig := scrapeConfig.(map[string]interface{})
if scrapeConfig["relabel_configs"] != nil {
relabelConfigs := scrapeConfig["relabel_configs"].([]interface{})
for _, relabelConfig := range relabelConfigs {
relabelConfig := relabelConfig.(map[string]interface{})
// Dropping regex key from the map since unmarshalling this on the client(metrics_receiver.go) results in error
// because of the bug here - https://github.com/prometheus/prometheus/issues/12534
if relabelConfig["action"] == "keepequal" || relabelConfig["action"] == "dropequal" {
delete(relabelConfig, "regex")
}
}
}
if scrapeConfig["metric_relabel_configs"] != nil {
metricRelabelConfigs := scrapeConfig["metric_relabel_configs"].([]interface{})
for _, metricRelabelConfig := range metricRelabelConfigs {
metricRelabelConfig := metricRelabelConfig.(map[string]interface{})
// Dropping regex key from the map since unmarshalling this on the client(metrics_receiver.go) results in error
// because of the bug here - https://github.com/prometheus/prometheus/issues/12534
if metricRelabelConfig["action"] == "keepequal" || metricRelabelConfig["action"] == "dropequal" {
delete(metricRelabelConfig, "regex")
}
}
}
}

jsonConfigNew, err := json.Marshal(jobToScrapeConfig)
if err != nil {
return nil, err
}
return jsonConfigNew, nil
}

// UpdateScrapeConfigResponse updates the scrape config response. The target allocator first marshals these
// configurations such that the underlying prometheus marshaling is used. After that, the YAML is converted
// in to a JSON format for consumers to use.
Expand All @@ -120,8 +165,14 @@ func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.Scrap
if err != nil {
return err
}

jsonConfigNew, err := RemoveRegexFromRelabelAction(jsonConfig)
if err != nil {
return err
}

s.mtx.Lock()
s.scrapeConfigResponse = jsonConfig
s.scrapeConfigResponse = jsonConfigNew
s.mtx.Unlock()
return nil
}
Expand Down
58 changes: 58 additions & 0 deletions cmd/otel-allocator/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"

"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation"
allocatorconfig "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config"
"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target"
)

Expand Down Expand Up @@ -612,6 +613,63 @@ func TestServer_Readiness(t *testing.T) {
}
}

func TestServer_ScrapeConfigRespose(t *testing.T) {
tests := []struct {
description string
filePath string
expectedCode int
}{
{
description: "Jobs with all actions",
filePath: "./testdata/prom-config-all-actions.yaml",
expectedCode: http.StatusOK,
},
{
description: "Jobs with config combinations",
filePath: "./testdata/prom-config-test.yaml",
expectedCode: http.StatusOK,
},
{
description: "Jobs with no config",
filePath: "./testdata/prom-no-config.yaml",
expectedCode: http.StatusOK,
},
}
for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {
listenAddr := ":8080"
s := NewServer(logger, nil, listenAddr)

allocCfg := allocatorconfig.CreateDefaultConfig()
err := allocatorconfig.LoadFromFile(tc.filePath, &allocCfg)
require.NoError(t, err)

jobToScrapeConfig := make(map[string]*promconfig.ScrapeConfig)

for _, scrapeConfig := range allocCfg.PromConfig.ScrapeConfigs {
jobToScrapeConfig[scrapeConfig.JobName] = scrapeConfig
}

assert.NoError(t, s.UpdateScrapeConfigResponse(jobToScrapeConfig))

request := httptest.NewRequest("GET", "/scrape_configs", nil)
w := httptest.NewRecorder()

s.server.Handler.ServeHTTP(w, request)
result := w.Result()

assert.Equal(t, tc.expectedCode, result.StatusCode)
bodyBytes, err := io.ReadAll(result.Body)
require.NoError(t, err)

// Checking to make sure yaml unmarshaling doesn't result in errors for responses containing any supported prometheus relabel action
scrapeConfigs := map[string]*promconfig.ScrapeConfig{}
err = yaml.Unmarshal(bodyBytes, scrapeConfigs)
require.NoError(t, err)
})
}
}

func newLink(jobName string) target.LinkJSON {
return target.LinkJSON{Link: fmt.Sprintf("/jobs/%s/targets", url.QueryEscape(jobName))}
}
175 changes: 175 additions & 0 deletions cmd/otel-allocator/server/testdata/prom-config-all-actions.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
collector_selector:
matchlabels:
app.kubernetes.io/instance: default.test
app.kubernetes.io/managed-by: opentelemetry-operator
prometheus_cr:
scrape_interval: 60s
config:
scrape_configs:
- job_name: job-replace
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_test]
action: replace
regex: "test$1replacement"
replacement: "myreplacement$$1"
target_label: "mytarget"
metric_relabel_configs:
- action: replace
source_labels: [city]
separator: ","
regex: (ci.*)(name$)
replacement: "test_newest_1_$1"
target_label: city
- job_name: job-lowercase
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_test]
action: lowercase
target_label: "mytarget"
metric_relabel_configs:
- action: lowercase
source_labels: [city]
target_label: city
- job_name: job-uppercase
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_test]
action: uppercase
target_label: "mytarget"
metric_relabel_configs:
- action: uppercase
source_labels: [city]
target_label: city
- job_name: job-keep
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
metric_relabel_configs:
- action: keep
source_labels: [city]
separator: ","
regex: (ci.*)(name$)
- job_name: job-drop
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_droplabel]
action: drop
regex: "prometheus-reference-app"
metric_relabel_configs:
- action: drop
source_labels: [city]
regex: (ci.*)(name$)
- job_name: job-keepequal
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keepequal
target_label: __meta_kubernetes_pod_label_mylabel
metric_relabel_configs:
- action: keepequal
source_labels: [city]
target_label: city
- job_name: job-dropequal
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_dropequallabel]
action: dropequal
target_label: "test"
metric_relabel_configs:
- action: dropequal
source_labels: [citytest]
target_label: city
- job_name: job-hashmod
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_hashmod]
action: hashmod
modulus: 5
regex: "prometheus-reference-app"
target_label: city
metric_relabel_configs:
- action: hashmod
modulus: 5
source_labels: [city]
regex: (ci.*)(name$)
target_label: city
- job_name: job-labelmap
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_hashmod]
action: labelmap
regex: "prometheus-reference-app"
target_label: city
metric_relabel_configs:
- action: labelmap
source_labels: [city]
regex: (ci.*)(name$)
target_label: city
- job_name: job-labeldrop
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- action: labeldrop
regex: "prometheus-reference-app"
metric_relabel_configs:
- action: labeldrop
regex: (ci.*)(name$)
- job_name: job-labelkeep
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- action: labelkeep
regex: "prometheus-reference-app"
metric_relabel_configs:
- action: labelkeep
regex: (ci.*)(name$$)
34 changes: 34 additions & 0 deletions cmd/otel-allocator/server/testdata/prom-config-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
collector_selector:
matchlabels:
app.kubernetes.io/instance: default.test
app.kubernetes.io/managed-by: opentelemetry-operator
prometheus_cr:
scrape_interval: 60s
config:
scrape_configs:
- job_name: job-no-target-relabel
scheme: http
kubernetes_sd_configs:
- role: pod
metric_relabel_configs:
- action: replace
source_labels: [city]
separator: ","
regex: (ci.*)(name$)
replacement: "test_newest_1_$1"
target_label: city
- job_name: job-no-metric-relabel
scheme: http
kubernetes_sd_configs:
- role: pod
relabel_configs:
- source_labels: [__meta_kubernetes_pod_label_app]
action: keep
regex: "prometheus-reference-app"
- source_labels: [__meta_kubernetes_pod_label_test]
action: lowercase
target_label: "mytarget"
- job_name: job-no-relabel
scheme: http
kubernetes_sd_configs:
- role: pod
8 changes: 8 additions & 0 deletions cmd/otel-allocator/server/testdata/prom-no-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
collector_selector:
matchlabels:
app.kubernetes.io/instance: default.test
app.kubernetes.io/managed-by: opentelemetry-operator
prometheus_cr:
scrape_interval: 60s
config:
scrape_configs:
Loading