Skip to content

Commit 95e8f8f

Browse files
aknuds1Aneurysm9
andauthored
prometheusremotewrite: Don't generate target_info unless at least one identifying label is defined (#32148)
**Description:** <Describe what has changed.> I propose modifying the Prometheus remote write translator such that it will only generate the `target_info` metric if at least one identifying label is defined, i.e. either `job` or `instance` (or both). The rationale is that without any identifying labels, `target_info` is useless (you can't join against it). See [Slack thread](https://cloud-native.slack.com/archives/C01LSCJBXDZ/p1712161732017409) for background. **Testing:** <Describe what testing was performed and which tests were added.> I've modified the `addResourceTargetInfo` tests to reflect that at least one identifying label is required, and added some new ones to cover the different cases. I also modify a couple of `prometheusremotewriteexporter` tests that are affected by this change, as they don't define the identifying resource attributes. **Documentation:** <Describe the documentation added.> --------- Signed-off-by: Arve Knudsen <[email protected]> Co-authored-by: Anthony Mirabella <[email protected]>
1 parent 06c970d commit 95e8f8f

File tree

4 files changed

+80
-40
lines changed

4 files changed

+80
-40
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: prometheusremotewrite
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Change prometheusremotewrite.FromMetrics so that the target_info metric is only generated if at least one identifying OTel resource attribute (service.name and/or service.instance.id) is defined.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [32148]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

exporter/prometheusremotewriteexporter/exporter_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ func Test_PushMetrics(t *testing.T) {
497497
name: "intSum_case",
498498
metrics: intSumBatch,
499499
reqTestFunc: checkFunc,
500-
expectedTimeSeries: 5,
500+
expectedTimeSeries: 4,
501501
httpResponseCode: http.StatusAccepted,
502502
},
503503
{

pkg/translator/prometheusremotewrite/helper.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,19 @@ func addResourceTargetInfo(resource pcommon.Resource, settings Settings, timesta
540540
}
541541

542542
labels := createAttributes(resource, attributes, settings.ExternalLabels, identifyingAttrs, false, model.MetricNameLabel, name)
543+
haveIdentifier := false
544+
for _, l := range labels {
545+
if l.Name == model.JobLabel || l.Name == model.InstanceLabel {
546+
haveIdentifier = true
547+
break
548+
}
549+
}
550+
551+
if !haveIdentifier {
552+
// We need at least one identifying label to generate target_info.
553+
return
554+
}
555+
543556
sample := &prompb.Sample{
544557
Value: float64(1),
545558
// convert ns to ms

pkg/translator/prometheusremotewrite/helper_test.go

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package prometheusremotewrite
55

66
import (
7-
"fmt"
87
"math"
98
"sort"
109
"testing"
@@ -532,10 +531,18 @@ func TestAddResourceTargetInfo(t *testing.T) {
532531
conventions.AttributeServiceInstanceID: "service-instance-id",
533532
}
534533
resourceWithServiceAttrs := pcommon.NewResource()
535-
assert.NoError(t, resourceWithServiceAttrs.Attributes().FromRaw(resourceAttrMap))
534+
require.NoError(t, resourceWithServiceAttrs.Attributes().FromRaw(resourceAttrMap))
536535
resourceWithServiceAttrs.Attributes().PutStr("resource_attr", "resource-attr-val-1")
537536
resourceWithOnlyServiceAttrs := pcommon.NewResource()
538-
assert.NoError(t, resourceWithOnlyServiceAttrs.Attributes().FromRaw(resourceAttrMap))
537+
require.NoError(t, resourceWithOnlyServiceAttrs.Attributes().FromRaw(resourceAttrMap))
538+
// service.name is an identifying resource attribute.
539+
resourceWithOnlyServiceName := pcommon.NewResource()
540+
resourceWithOnlyServiceName.Attributes().PutStr(conventions.AttributeServiceName, "service-name")
541+
resourceWithOnlyServiceName.Attributes().PutStr("resource_attr", "resource-attr-val-1")
542+
// service.instance.id is an identifying resource attribute.
543+
resourceWithOnlyServiceID := pcommon.NewResource()
544+
resourceWithOnlyServiceID.Attributes().PutStr(conventions.AttributeServiceInstanceID, "service-instance-id")
545+
resourceWithOnlyServiceID.Attributes().PutStr("resource_attr", "resource-attr-val-1")
539546
for _, tc := range []struct {
540547
desc string
541548
resource pcommon.Resource
@@ -549,61 +556,54 @@ func TestAddResourceTargetInfo(t *testing.T) {
549556
},
550557
{
551558
desc: "disable target info metric",
552-
resource: testdata.GenerateMetricsNoLibraries().ResourceMetrics().At(0).Resource(),
559+
resource: resourceWithOnlyServiceName,
553560
settings: Settings{DisableTargetInfo: true},
554561
},
555562
{
556-
desc: "with resource",
563+
desc: "with resource missing both service.name and service.instance.id resource attributes",
557564
resource: testdata.GenerateMetricsNoLibraries().ResourceMetrics().At(0).Resource(),
558565
timestamp: testdata.TestMetricStartTimestamp,
566+
},
567+
{
568+
desc: "with resource including service.instance.id, and missing service.name resource attribute",
569+
resource: resourceWithOnlyServiceID,
570+
timestamp: testdata.TestMetricStartTimestamp,
559571
wantLabels: []prompb.Label{
560-
{
561-
Name: model.MetricNameLabel,
562-
Value: targetMetricName,
563-
},
564-
{
565-
Name: "resource_attr",
566-
Value: "resource-attr-val-1",
567-
},
572+
{Name: model.MetricNameLabel, Value: "target_info"},
573+
{Name: model.InstanceLabel, Value: "service-instance-id"},
574+
{Name: "resource_attr", Value: "resource-attr-val-1"},
568575
},
569576
},
570577
{
571-
desc: "with resource, with namespace",
572-
resource: testdata.GenerateMetricsNoLibraries().ResourceMetrics().At(0).Resource(),
578+
desc: "with resource including service.name, and missing service.instance.id resource attribute",
579+
resource: resourceWithOnlyServiceName,
580+
timestamp: testdata.TestMetricStartTimestamp,
581+
wantLabels: []prompb.Label{
582+
{Name: model.MetricNameLabel, Value: "target_info"},
583+
{Name: model.JobLabel, Value: "service-name"},
584+
{Name: "resource_attr", Value: "resource-attr-val-1"},
585+
},
586+
},
587+
{
588+
desc: "with valid resource, with namespace",
589+
resource: resourceWithOnlyServiceName,
573590
timestamp: testdata.TestMetricStartTimestamp,
574591
settings: Settings{Namespace: "foo"},
575592
wantLabels: []prompb.Label{
576-
{
577-
Name: model.MetricNameLabel,
578-
Value: fmt.Sprintf("foo_%s", targetMetricName),
579-
},
580-
{
581-
Name: "resource_attr",
582-
Value: "resource-attr-val-1",
583-
},
593+
{Name: model.MetricNameLabel, Value: "foo_target_info"},
594+
{Name: model.JobLabel, Value: "service-name"},
595+
{Name: "resource_attr", Value: "resource-attr-val-1"},
584596
},
585597
},
586598
{
587599
desc: "with resource, with service attributes",
588600
resource: resourceWithServiceAttrs,
589601
timestamp: testdata.TestMetricStartTimestamp,
590602
wantLabels: []prompb.Label{
591-
{
592-
Name: model.MetricNameLabel,
593-
Value: targetMetricName,
594-
},
595-
{
596-
Name: "instance",
597-
Value: "service-instance-id",
598-
},
599-
{
600-
Name: "job",
601-
Value: "service-namespace/service-name",
602-
},
603-
{
604-
Name: "resource_attr",
605-
Value: "resource-attr-val-1",
606-
},
603+
{Name: model.MetricNameLabel, Value: "target_info"},
604+
{Name: model.InstanceLabel, Value: "service-instance-id"},
605+
{Name: model.JobLabel, Value: "service-namespace/service-name"},
606+
{Name: "resource_attr", Value: "resource-attr-val-1"},
607607
},
608608
},
609609
{

0 commit comments

Comments
 (0)