Require for either job or instance label to be defined

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
open-telemetry · Apr 4, 2024 · d943bf8 · d943bf8
1 parent 89c64af
commit d943bf8
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 27 deletions.
diff --git a/.chloggen/bugfix_target-info-required-attrs.yaml b/.chloggen/bugfix_target-info-required-attrs.yaml
@@ -7,15 +7,15 @@ change_type: bug_fix
 component: prometheusremotewrite
 
 # A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
-note: Change prometheusremotewrite.FromMetrics so that the target_info metric is only generated if the OTel resource attribute service.name is defined.
+note: Change prometheusremotewrite.FromMetrics so that the target_info metric is only generated if either of the OTel resource attributes service.name and service.instance.id is defined.
 
 # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
 issues: [32148]
 
 # (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: The resource attribute service.name is mandatory according to the OTel spec.
+subtext:
 
 # If your change doesn't affect end users or the exported elements of any package,
 # you should instead start your pull request title with [chore] or use the "Skip Changelog" label.

diff --git a/pkg/translator/prometheusremotewrite/helper.go b/pkg/translator/prometheusremotewrite/helper.go
@@ -565,19 +565,10 @@ func addResourceTargetInfo(resource pcommon.Resource, settings Settings, timesta
 		}
 	}
 
-	if !haveJob {
-		// The resource attribute service.name is mandatory according to OTel spec.
-		// If it's missing, don't generate target_info.
+	if !haveJob && !haveInstance {
+		// We need at least one identifying label to generate target_info.
 		return
 	}
-	if !haveInstance {
-		// The resource attribute service.instance.id is optional, but let's just be explicit
-		// as ideally the target info should also be identified through the instance label.
-		labels = append(labels, prompb.Label{
-			Name:  model.InstanceLabel,
-			Value: "",
-		})
-	}
 
 	sample := &prompb.Sample{
 		Value: float64(1),

diff --git a/pkg/translator/prometheusremotewrite/helper_test.go b/pkg/translator/prometheusremotewrite/helper_test.go
@@ -515,11 +515,11 @@ func TestAddResourceTargetInfo(t *testing.T) {
 	resourceWithServiceAttrs.Attributes().PutStr("resource_attr", "resource-attr-val-1")
 	resourceWithOnlyServiceAttrs := pcommon.NewResource()
 	require.NoError(t, resourceWithOnlyServiceAttrs.Attributes().FromRaw(resourceAttrMap))
-	// service.name is the one mandatory resource attribute.
+	// service.name is an identifying resource attribute.
 	resourceWithOnlyServiceName := pcommon.NewResource()
 	resourceWithOnlyServiceName.Attributes().PutStr(conventions.AttributeServiceName, "service-name")
 	resourceWithOnlyServiceName.Attributes().PutStr("resource_attr", "resource-attr-val-1")
-	// service.instance.id is an identifying resource attribute, but not mandatory
+	// service.instance.id is an identifying resource attribute.
 	resourceWithOnlyServiceID := pcommon.NewResource()
 	resourceWithOnlyServiceID.Attributes().PutStr(conventions.AttributeServiceInstanceID, "service-instance-id")
 	resourceWithOnlyServiceID.Attributes().PutStr("resource_attr", "resource-attr-val-1")
@@ -548,26 +548,45 @@ func TestAddResourceTargetInfo(t *testing.T) {
 			expected:  map[string]*prompb.TimeSeries{},
 		},
 		{
-			desc:      "with resource including service.instance.id, but missing service.name resource attribute",
+			desc:      "with resource including service.instance.id, and missing service.name resource attribute",
 			resource:  resourceWithOnlyServiceID,
 			timestamp: testdata.TestMetricStartTimestamp,
-			expected:  map[string]*prompb.TimeSeries{},
+			expected: map[string]*prompb.TimeSeries{
+				"info-__name__-target_info-instance-service-instance-id-resource_attr-resource-attr-val-1": {
+					Labels: []prompb.Label{
+						{
+							Name:  model.MetricNameLabel,
+							Value: "target_info",
+						},
+						{
+							Name:  model.InstanceLabel,
+							Value: "service-instance-id",
+						},
+						{
+							Name:  "resource_attr",
+							Value: "resource-attr-val-1",
+						},
+					},
+					Samples: []prompb.Sample{
+						{
+							Value:     1,
+							Timestamp: 1581452772000,
+						},
+					},
+				},
+			},
 		},
 		{
 			desc:      "with resource including service.name, and missing service.instance.id resource attribute",
 			resource:  resourceWithOnlyServiceName,
 			timestamp: testdata.TestMetricStartTimestamp,
 			expected: map[string]*prompb.TimeSeries{
-				"info-__name__-target_info-instance--job-service-name-resource_attr-resource-attr-val-1": {
+				"info-__name__-target_info-job-service-name-resource_attr-resource-attr-val-1": {
 					Labels: []prompb.Label{
 						{
 							Name:  model.MetricNameLabel,
 							Value: "target_info",
 						},
-						{
-							Name:  model.InstanceLabel,
-							Value: "",
-						},
 						{
 							Name:  model.JobLabel,
 							Value: "service-name",
@@ -592,16 +611,12 @@ func TestAddResourceTargetInfo(t *testing.T) {
 			timestamp: testdata.TestMetricStartTimestamp,
 			settings:  Settings{Namespace: "foo"},
 			expected: map[string]*prompb.TimeSeries{
-				"info-__name__-foo_target_info-instance--job-service-name-resource_attr-resource-attr-val-1": {
+				"info-__name__-foo_target_info-job-service-name-resource_attr-resource-attr-val-1": {
 					Labels: []prompb.Label{
 						{
 							Name:  model.MetricNameLabel,
 							Value: "foo_target_info",
 						},
-						{
-							Name:  model.InstanceLabel,
-							Value: "",
-						},
 						{
 							Name:  model.JobLabel,
 							Value: "service-name",