Skip to content

Commit

Permalink
Fix processscraper resource attribute test
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
braydonk committed Nov 25, 2024
1 parent 5698c9d commit cb18dfb
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
}

Expand Down
5 changes: 0 additions & 5 deletions receiver/hostmetricsreceiver/internal/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit cb18dfb

Please sign in to comment.