From cb18dfb4ace0478d97ef6f95585435335f8dceeb Mon Sep 17 00:00:00 2001
From: braydonk <braydonk@google.com>
Date: Mon, 25 Nov 2024 21:57:05 +0000
Subject: [PATCH] Fix processscraper resource attribute test

Previously, this test was repeatedly testing the first process in the
resource. This tended to just work, but is not testing what it is
supposed to. When changing it to test every resource in the scrape, it
came up with issues due to the fact that not all resource attributes can
always be found.

This PR changes the test to look at all process resources in the scrape,
and instead of asserting the presence of every resource attribute, it
checks for one mandatory attribute (the process ID) and ensures that no
unexpected resource attributes are added. This should reduce the
flakiness of this test on other environments, while still asserting this
on every process resource in the list.
---
 .../processscraper/process_scraper_test.go    | 35 +++++++++++++------
 .../hostmetricsreceiver/internal/testutils.go |  5 ---
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go
index e12f8f2ca545..058b434e5cec 100644
--- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go
+++ b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go
@@ -125,7 +125,7 @@ func TestScrape(t *testing.T) {
 			}
 
 			require.Greater(t, md.ResourceMetrics().Len(), 1)
-			assertProcessResourceAttributesExist(t, md.ResourceMetrics())
+			assertValidProcessResourceAttributes(t, md.ResourceMetrics())
 			assertCPUTimeMetricValid(t, md.ResourceMetrics(), expectedStartTime)
 			if metricsBuilderConfig.Metrics.ProcessCPUUtilization.Enabled {
 				assertCPUUtilizationMetricValid(t, md.ResourceMetrics(), expectedStartTime)
@@ -170,16 +170,31 @@ func TestScrape(t *testing.T) {
 	}
 }
 
-func assertProcessResourceAttributesExist(t *testing.T, resourceMetrics pmetric.ResourceMetricsSlice) {
+// assertValidProcessResourceAttributes will assert that each process resource contains at least
+// the most important identifying attribute (the PID attribute) and only contains permissible
+// resource attributes defined by the process scraper metadata.yaml/Process Semantic Conventions.
+func assertValidProcessResourceAttributes(t *testing.T, resourceMetrics pmetric.ResourceMetricsSlice) {
+	requiredResourceAttributes := []string{
+		conventions.AttributeProcessPID,
+	}
+	permissibleResourceAttributes := []string{
+		conventions.AttributeProcessPID,
+		conventions.AttributeProcessExecutableName,
+		conventions.AttributeProcessExecutablePath,
+		conventions.AttributeProcessCommand,
+		conventions.AttributeProcessCommandLine,
+		conventions.AttributeProcessOwner,
+		"process.parent_pid", // TODO: use this from conventions when it is available
+	}
 	for i := 0; i < resourceMetrics.Len(); i++ {
-		attr := resourceMetrics.At(0).Resource().Attributes()
-		internal.AssertContainsAttribute(t, attr, conventions.AttributeProcessPID)
-		internal.AssertContainsAttribute(t, attr, conventions.AttributeProcessExecutableName)
-		internal.AssertContainsAttribute(t, attr, conventions.AttributeProcessExecutablePath)
-		internal.AssertContainsAttribute(t, attr, conventions.AttributeProcessCommand)
-		internal.AssertContainsAttribute(t, attr, conventions.AttributeProcessCommandLine)
-		internal.AssertContainsAttribute(t, attr, conventions.AttributeProcessOwner)
-		internal.AssertContainsAttribute(t, attr, "process.parent_pid")
+		attrs := resourceMetrics.At(i).Resource().Attributes().AsRaw()
+		for _, attr := range requiredResourceAttributes {
+			_, ok := attrs[attr]
+			require.True(t, ok, "resource %s missing required attribute %s", attrs, attr)
+		}
+		for attr := range attrs {
+			require.Contains(t, permissibleResourceAttributes, attr, "resource %s contained unknown attribute %s", attrs, attr)
+		}
 	}
 }
 
diff --git a/receiver/hostmetricsreceiver/internal/testutils.go b/receiver/hostmetricsreceiver/internal/testutils.go
index 7c7d82a4bcd5..3a34bc182989 100644
--- a/receiver/hostmetricsreceiver/internal/testutils.go
+++ b/receiver/hostmetricsreceiver/internal/testutils.go
@@ -12,11 +12,6 @@ import (
 	"go.opentelemetry.io/collector/pdata/pmetric"
 )
 
-func AssertContainsAttribute(t *testing.T, attr pcommon.Map, key string) {
-	_, ok := attr.Get(key)
-	assert.True(t, ok)
-}
-
 func AssertDescriptorEqual(t *testing.T, expected pmetric.Metric, actual pmetric.Metric) {
 	assert.Equal(t, expected.Name(), actual.Name())
 	assert.Equal(t, expected.Description(), actual.Description())