Skip to content

Commit 3114c9b

Browse files
authored
[AGTHEAL-223] http-sd: apply check template rename_labels to SD target tags (#53774)
### What does this PR do? This PR makes the OpenMetrics `rename_labels` configuration apply consistently to labels coming from Prometheus HTTP Service Discovery. The HTTP SD provider converts labels returned by the service discovery endpoint into instance tags on the generated OpenMetrics check. Because those tags are added outside the OpenMetrics scraping pipeline, they previously bypassed rename_labels, which only handled labels found in the scraped metrics payload. This change extracts the existing rename_labels map from the check template and applies it to HTTP SD labels before converting them into tags. No new configuration is introduced. For example: ``` prometheus_http_sd: configs: - url: "http://my-sd-service:8080/service_instances" check_template: | { "name": "openmetrics", "init_config": {}, "instances": [ { "openmetrics_endpoint": "http://%%host%%:%%port%%/metrics", "namespace": "app", "metrics": [".*"], "rename_labels": { "experiment": "appXYZ.experiment" } } ] } ``` An HTTP SD response containing: ``` labels: experiment: test123 ``` now produces tag: `appXYZ.experiment:test123` ### Motivation Previously, `rename_labels` in OM check was only applied to labels parsed from the scraped `/metrics` payload. Labels provided by the HTTP Service Discovery endpoint were converted directly into instance tags by the provider and therefore bypassed the same renaming logic. The difference came from where the labels entered the generated check: scraped metric labels were processed by the OpenMetrics check HTTP SD labels were converted directly into instance tags by the provider This PR closes that gap so a single `rename_labels` configuration applies consistently to both scraped metric labels and HTTP SD target labels. ### Describe how you validated your changes ``` clusterAgent: enabled: true env: - name: DD_PROMETHEUS_HTTP_SD_URL value: "http://my-sd-service:8080/service_instances" - name: DD_PROMETHEUS_HTTP_SD_CHECK_TEMPLATE value: '{"name":"openmetrics","init_config":{},"instances":[{"openmetrics_endpoint":"http://%%host%%:%%port%%/metrics","namespace":"app","metrics":[".*"],"rename_labels":{"experiment":"appXYZ.experiment"}}]}' ``` With the SD service returning: ``` [ { "targets": ["10.0.10.2:9100", "10.0.10.3:9100", "10.0.10.4:9100", "10.0.10.5:9100"], "labels": { "experiment":"test123" } } ... ] ``` the generated OM check gets tag `appXYZ.experiment:test123` (renamed). ### Tag de-duplication behavior `rename_labels` in the provider can rename an SD label to a key that also arrives from scraped /metrics endpoint.The [de-duplication process](https://github.com/DataDog/datadog-agent/blob/9b52f96ec484f74e39d4020c57b77d1c04972e52/pkg/tagset/hash_generator.go#L10-L16) is done by the aggregator when it builds the metric's context key. For instance, there are two identical renamed tags from SD and scraped metrics endpoint, the aggregator will collapse the identical strings before sending to backend. Co-authored-by: minyi.zhu <minyi.zhu@datadoghq.com>
1 parent 643d479 commit 3114c9b

3 files changed

Lines changed: 180 additions & 4 deletions

File tree

comp/core/autodiscovery/providers/prometheus_http_sd.go

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ type httpSDEntry struct {
6969
client *http.Client
7070
checkTemplate httpSDCheckTemplate
7171
filterProgram cel.Program // compiled exclude_filter CEL program; nil if no filter
72+
// renameLabels mirrors the check template's openmetrics "rename_labels" map.
73+
// It is applied to the SD target labels before they are injected as instance
74+
// tags, so the same rename_labels the user configures for scraped metrics also
75+
// covers the tags derived from the SD response. nil if the template has none.
76+
renameLabels map[string]string
7277
}
7378

7479
// httpSDConfigEntry mirrors a single entry under prometheus_http_sd.configs in
@@ -113,6 +118,7 @@ func buildEntries(rawConfigs []httpSDConfigEntry, sharedClient *http.Client) ([]
113118
client: sharedClient,
114119
checkTemplate: tmpl,
115120
filterProgram: filterProg,
121+
renameLabels: extractRenameLabels(tmpl),
116122
})
117123
}
118124
return entries, errs
@@ -269,7 +275,7 @@ func (e *httpSDEntry) collect() ([]integration.Config, error) {
269275

270276
var configs []integration.Config
271277
for _, tg := range targetGroups {
272-
tags := labelsToTags(tg.Labels)
278+
tags := labelsToTags(tg.Labels, e.renameLabels)
273279

274280
for _, target := range tg.Targets {
275281
host, port, splitErr := net.SplitHostPort(target)
@@ -359,8 +365,13 @@ func (e *httpSDEntry) buildConfig(host, port string, tags []string) (integration
359365

360366
// labelsToTags converts HTTP SD labels to Datadog tags.
361367
// Internal labels (prefixed with __) except __meta_ are skipped.
368+
// When renameLabels is non-empty, a tag key matching one of its entries is
369+
// renamed, mirroring the openmetrics check's rename_labels behavior so the same
370+
// mapping applies to both scraped labels and SD-derived tags. The lookup uses
371+
// the effective tag key (after the __meta_ prefix is stripped), which is the
372+
// name the user sees as a tag.
362373
// Tags are sorted for stable config digests across polls.
363-
func labelsToTags(labels map[string]string) []string {
374+
func labelsToTags(labels, renameLabels map[string]string) []string {
364375
var tags []string
365376
for k, v := range labels {
366377
tagKey := k
@@ -369,12 +380,40 @@ func labelsToTags(labels map[string]string) []string {
369380
} else if strings.HasPrefix(k, "__") {
370381
continue
371382
}
383+
if renamed, ok := renameLabels[tagKey]; ok {
384+
tagKey = renamed
385+
}
372386
tags = append(tags, tagKey+":"+v)
373387
}
374388
sort.Strings(tags)
375389
return tags
376390
}
377391

392+
// extractRenameLabels reads the openmetrics "rename_labels" map from the first
393+
// instance of the check template, if present. The provider is otherwise
394+
// check-agnostic; this reaches into the openmetrics schema intentionally so the
395+
// SD-derived tags honor the same rename_labels the user already configures for
396+
// scraped metrics. Returns nil when the field is absent or malformed.
397+
func extractRenameLabels(tmpl httpSDCheckTemplate) map[string]string {
398+
if len(tmpl.Instances) == 0 {
399+
return nil
400+
}
401+
raw, ok := tmpl.Instances[0]["rename_labels"].(map[string]interface{})
402+
if !ok {
403+
return nil
404+
}
405+
renameLabels := make(map[string]string, len(raw))
406+
for k, v := range raw {
407+
if s, ok := v.(string); ok {
408+
renameLabels[k] = s
409+
}
410+
}
411+
if len(renameLabels) == 0 {
412+
return nil
413+
}
414+
return renameLabels
415+
}
416+
378417
// substituteTemplateVars replaces %%host%% and %%port%% placeholders in a value.
379418
// Only string values are substituted; all other types are returned as-is.
380419
func substituteTemplateVars(v interface{}, host, port string) interface{} {

comp/core/autodiscovery/providers/prometheus_http_sd_test.go

Lines changed: 129 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ func makeTestProviderWithEntries(t *testing.T, specs []entrySpec) *PrometheusHTT
4747
client: http.DefaultClient,
4848
checkTemplate: tmpl,
4949
filterProgram: filterProg,
50+
renameLabels: extractRenameLabels(tmpl),
5051
}
5152
}
5253

@@ -180,7 +181,7 @@ func TestLabelsToTags(t *testing.T) {
180181

181182
for _, tt := range tests {
182183
t.Run(tt.name, func(t *testing.T) {
183-
tags := labelsToTags(tt.labels)
184+
tags := labelsToTags(tt.labels, nil)
184185
assert.Equal(t, tt.expected, tags)
185186
})
186187
}
@@ -195,7 +196,7 @@ func TestLabelsToTagsStableOrder(t *testing.T) {
195196

196197
// Run multiple times to verify deterministic ordering
197198
for i := 0; i < 10; i++ {
198-
tags := labelsToTags(labels)
199+
tags := labelsToTags(labels, nil)
199200
assert.Equal(t, []string{"a_label:first", "m_label:middle", "z_label:last"}, tags)
200201
}
201202
}
@@ -792,3 +793,129 @@ func TestNewPrometheusHTTPSDConfigProviderFromConfig(t *testing.T) {
792793
require.NoError(t, yaml.Unmarshal(configs[0].Instances[0], &instance))
793794
assert.Equal(t, "http://host2:9100/metrics", instance["openmetrics_endpoint"])
794795
}
796+
797+
func TestLabelsToTagsRename(t *testing.T) {
798+
tests := []struct {
799+
name string
800+
labels map[string]string
801+
renameLabels map[string]string
802+
expected []string
803+
}{
804+
{
805+
name: "plain label is renamed",
806+
labels: map[string]string{"experiment": "test123", "service": "node"},
807+
renameLabels: map[string]string{"experiment": "appXYZ.experiment"},
808+
expected: []string{"appXYZ.experiment:test123", "service:node"},
809+
},
810+
{
811+
name: "rename applies to __meta_ label after prefix is stripped",
812+
labels: map[string]string{"__meta_service_type": "worker"},
813+
renameLabels: map[string]string{"service_type": "appXYZ.service_type"},
814+
expected: []string{"appXYZ.service_type:worker"},
815+
},
816+
{
817+
name: "label not in rename map is untouched",
818+
labels: map[string]string{"user": "alice"},
819+
renameLabels: map[string]string{"experiment": "appXYZ.experiment"},
820+
expected: []string{"user:alice"},
821+
},
822+
{
823+
name: "nil rename map is a no-op",
824+
labels: map[string]string{"experiment": "test123"},
825+
renameLabels: nil,
826+
expected: []string{"experiment:test123"},
827+
},
828+
{
829+
name: "__ internal labels stay skipped even if named in the map",
830+
labels: map[string]string{"__address__": "10.0.0.1:9090", "experiment": "test123"},
831+
renameLabels: map[string]string{"experiment": "appXYZ.experiment"},
832+
expected: []string{"appXYZ.experiment:test123"},
833+
},
834+
}
835+
836+
for _, tt := range tests {
837+
t.Run(tt.name, func(t *testing.T) {
838+
tags := labelsToTags(tt.labels, tt.renameLabels)
839+
assert.Equal(t, tt.expected, tags)
840+
})
841+
}
842+
}
843+
844+
func TestExtractRenameLabels(t *testing.T) {
845+
tests := []struct {
846+
name string
847+
template string
848+
expected map[string]string
849+
}{
850+
{
851+
name: "rename_labels present",
852+
template: `{"name":"openmetrics","instances":[{"rename_labels":{"experiment":"appXYZ.experiment","pool":"appXYZ.pool"}}]}`,
853+
expected: map[string]string{"experiment": "appXYZ.experiment", "pool": "appXYZ.pool"},
854+
},
855+
{
856+
name: "no rename_labels field",
857+
template: `{"name":"openmetrics","instances":[{"openmetrics_endpoint":"http://x/metrics"}]}`,
858+
expected: nil,
859+
},
860+
{
861+
name: "empty rename_labels map",
862+
template: `{"name":"openmetrics","instances":[{"rename_labels":{}}]}`,
863+
expected: nil,
864+
},
865+
{
866+
name: "non-string values are skipped",
867+
template: `{"name":"openmetrics","instances":[{"rename_labels":{"experiment":"appXYZ.experiment","bad":123}}]}`,
868+
expected: map[string]string{"experiment": "appXYZ.experiment"},
869+
},
870+
}
871+
872+
for _, tt := range tests {
873+
t.Run(tt.name, func(t *testing.T) {
874+
var tmpl httpSDCheckTemplate
875+
require.NoError(t, json.Unmarshal([]byte(tt.template), &tmpl))
876+
assert.Equal(t, tt.expected, extractRenameLabels(tmpl))
877+
})
878+
}
879+
}
880+
881+
// TestRenameLabelsAppliedToSDTags is the end-to-end case behind CONTP-1801:
882+
// the check template's rename_labels must also rename the tags derived from the
883+
// SD target labels, not just scraped metrics. Here the SD response labels the
884+
// target with `experiment`, which the OpenMetrics check alone could never rename
885+
// (it's an injected instance tag, not a scraped label).
886+
func TestRenameLabelsAppliedToSDTags(t *testing.T) {
887+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
888+
json.NewEncoder(w).Encode([]httpSDTargetGroup{
889+
{
890+
Targets: []string{"10.0.0.7:8000"},
891+
Labels: map[string]string{"experiment": "test123", "service": "node"},
892+
},
893+
})
894+
}))
895+
defer server.Close()
896+
897+
provider := makeTestProviderWithEntries(t, []entrySpec{{
898+
url: server.URL,
899+
template: `{"name":"openmetrics","init_config":{},"instances":[{"openmetrics_endpoint":"http://%%host%%:%%port%%/metrics","rename_labels":{"experiment":"appXYZ.experiment"}}]}`,
900+
}})
901+
902+
configs, err := provider.Collect(context.Background())
903+
require.NoError(t, err)
904+
require.Len(t, configs, 1)
905+
906+
var instance map[string]interface{}
907+
require.NoError(t, yaml.Unmarshal(configs[0].Instances[0], &instance))
908+
909+
tags := make([]string, 0)
910+
for _, tag := range instance["tags"].([]interface{}) {
911+
tags = append(tags, tag.(string))
912+
}
913+
assert.Contains(t, tags, "appXYZ.experiment:test123", "SD `experiment` label should be renamed via rename_labels")
914+
assert.NotContains(t, tags, "experiment:test123", "raw `experiment` tag should not remain after rename")
915+
assert.Contains(t, tags, "service:node", "unmapped SD labels should be untouched")
916+
917+
// rename_labels stays in the instance so the OpenMetrics check still renames
918+
// scraped labels of the same name. (YAML v2 decodes nested maps with
919+
// interface{} keys.)
920+
assert.Equal(t, map[interface{}]interface{}{"experiment": "appXYZ.experiment"}, instance["rename_labels"])
921+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
enhancements:
3+
- |
4+
The Cluster Agent's Prometheus HTTP Service Discovery provider now applies
5+
the OpenMetrics check template's ``rename_labels`` mapping to the tags
6+
derived from the SD target labels, in addition to the labels scraped from
7+
each target. Previously ``rename_labels`` only affected scraped metric
8+
labels, so a label supplied by the SD endpoint could not be renamed.
9+
No configuration change is required: the existing
10+
``rename_labels`` in the ``check_template`` now covers both sources.

0 commit comments

Comments
 (0)