Skip to content

Commit

Permalink
[receiver/hostmetrics] send process.cpu.utilization if enabled (open-…
Browse files Browse the repository at this point in the history
…telemetry#23451)

Currently `process.cpu.utilization` is not sent when it is enabled if
`process.cpu.time` is disabled.
It should be sent independently of `process.cpu.time`
  • Loading branch information
rubenruizdegauna authored and fchikwekwe committed Jun 23, 2023
1 parent c801cee commit d45156a
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 2 deletions.
20 changes: 20 additions & 0 deletions .chloggen/fix-hostmetrics-receiver-process-cpu-utilization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Use this changelog template to create an entry for release notes.
# If your change doesn't affect end users, such as a test fix or a tooling change,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: receiver/hostmetricsreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix not sending `process.cpu.utilization` when `process.cpu.time` is disabled.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [23450]

# (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:
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (s *scraper) getProcessMetadata() ([]*processMetadata, error) {
}

func (s *scraper) scrapeAndAppendCPUTimeMetric(now pcommon.Timestamp, handle processHandle, pid int32) error {
if !s.config.MetricsBuilderConfig.Metrics.ProcessCPUTime.Enabled {
if !s.config.MetricsBuilderConfig.Metrics.ProcessCPUTime.Enabled && !s.config.MetricsBuilderConfig.Metrics.ProcessCPUUtilization.Enabled {
return nil
}

Expand All @@ -246,7 +246,14 @@ func (s *scraper) scrapeAndAppendCPUTimeMetric(now pcommon.Timestamp, handle pro
return err
}

s.recordCPUTimeMetric(now, times)
if s.config.MetricsBuilderConfig.Metrics.ProcessCPUTime.Enabled {
s.recordCPUTimeMetric(now, times)
}

if !s.config.MetricsBuilderConfig.Metrics.ProcessCPUUtilization.Enabled {
return nil
}

if _, ok := s.ucals[pid]; !ok {
s.ucals[pid] = &ucal.CPUUtilizationCalculator{}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1089,3 +1089,85 @@ func TestScrapeMetrics_DontCheckDisabledMetrics(t *testing.T) {
assert.Nil(t, err)
})
}

func TestScrapeMetrics_CpuUtilizationWhenCpuTimesIsDisabled(t *testing.T) {
skipTestOnUnsupportedOS(t)

testCases := []struct {
name string
processCPUTimes bool
processCPUUtilization bool
expectedMetricCount int
expectedMetricNames []string
}{
{
name: "process.cpu.time enabled, process.cpu.utilization disabled",
processCPUTimes: true,
processCPUUtilization: false,
expectedMetricCount: 1,
expectedMetricNames: []string{"process.cpu.time"},
},
{
name: "process.cpu.time disabled, process.cpu.utilization enabled",
processCPUTimes: false,
processCPUUtilization: true,
expectedMetricCount: 1,
expectedMetricNames: []string{"process.cpu.utilization"},
},
{
name: "process.cpu.time enabled, process.cpu.utilization enabled",
processCPUTimes: true,
processCPUUtilization: true,
expectedMetricCount: 2,
expectedMetricNames: []string{"process.cpu.time", "process.cpu.utilization"},
},
}

for i := range testCases {
testCase := testCases[i]
t.Run(testCase.name, func(t *testing.T) {
metricsBuilderConfig := metadata.DefaultMetricsBuilderConfig()

metricsBuilderConfig.Metrics.ProcessCPUTime.Enabled = testCase.processCPUTimes
metricsBuilderConfig.Metrics.ProcessCPUUtilization.Enabled = testCase.processCPUUtilization

// disable some default metrics for easy assertion
metricsBuilderConfig.Metrics.ProcessMemoryUsage.Enabled = false
metricsBuilderConfig.Metrics.ProcessMemoryVirtual.Enabled = false
metricsBuilderConfig.Metrics.ProcessDiskIo.Enabled = false

config := &Config{MetricsBuilderConfig: metricsBuilderConfig}

scraper, err := newProcessScraper(receivertest.NewNopCreateSettings(), config)
require.NoError(t, err, "Failed to create process scraper: %v", err)
err = scraper.start(context.Background(), componenttest.NewNopHost())
require.NoError(t, err, "Failed to initialize process scraper: %v", err)

handleMock := newDefaultHandleMock()
handleMock.On("Name").Return("test", nil)
handleMock.On("Exe").Return("test", nil)
handleMock.On("CreateTime").Return(time.Now().UnixMilli(), nil)

scraper.getProcessHandles = func() (processHandles, error) {
return &processHandlesMock{handles: []*processHandleMock{handleMock}}, nil
}

// scrape the first time
_, err = scraper.scrape(context.Background())
assert.Nil(t, err)

// scrape second time to get utilization
md, err := scraper.scrape(context.Background())
assert.Nil(t, err)

for k := 0; k < md.ResourceMetrics().At(0).ScopeMetrics().At(0).Metrics().Len(); k++ {
fmt.Println(md.ResourceMetrics().At(0).ScopeMetrics().At(0).Metrics().At(k).Name())
}
assert.Equal(t, testCase.expectedMetricCount, md.MetricCount())
for metricIdx, expectedMetricName := range testCase.expectedMetricNames {
assert.Equal(t, expectedMetricName, md.ResourceMetrics().At(0).ScopeMetrics().At(0).Metrics().At(metricIdx).Name())
}
})
}

}

0 comments on commit d45156a

Please sign in to comment.