From c79f50b846494f488df45e7b6505e91868560fcd Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Wed, 5 Aug 2020 09:54:40 -0700 Subject: [PATCH] Add process telemetry tests and avoid frequent error on Windows (#1487) * Add process telemetry tests * Get the procfs.Proc instance at creation, this avoids failing constantly on Windows. --- internal/collector/telemetry/empty_test.go | 15 ---- .../collector/telemetry/process_telemetry.go | 25 +++++-- .../telemetry/process_telemetry_test.go | 73 +++++++++++++++++++ 3 files changed, 90 insertions(+), 23 deletions(-) delete mode 100644 internal/collector/telemetry/empty_test.go create mode 100644 internal/collector/telemetry/process_telemetry_test.go diff --git a/internal/collector/telemetry/empty_test.go b/internal/collector/telemetry/empty_test.go deleted file mode 100644 index 7a3772fcc150..000000000000 --- a/internal/collector/telemetry/empty_test.go +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package telemetry diff --git a/internal/collector/telemetry/process_telemetry.go b/internal/collector/telemetry/process_telemetry.go index d2e9d6f7b259..fe0240640b1a 100644 --- a/internal/collector/telemetry/process_telemetry.go +++ b/internal/collector/telemetry/process_telemetry.go @@ -30,6 +30,7 @@ type ProcessMetricsViews struct { ballastSizeBytes uint64 views []*view.View done chan struct{} + proc *procfs.Proc } var mRuntimeAllocMem = stats.Int64( @@ -83,17 +84,27 @@ var viewCPUSeconds = &view.View{ // NewProcessMetricsViews creates a new set of ProcessMetrics (mem, cpu) that can be used to measure // basic information about this process. func NewProcessMetricsViews(ballastSizeBytes uint64) *ProcessMetricsViews { - return &ProcessMetricsViews{ + pmv := &ProcessMetricsViews{ ballastSizeBytes: ballastSizeBytes, views: []*view.View{viewAllocMem, viewTotalAllocMem, viewSysMem, viewCPUSeconds}, done: make(chan struct{}), } + + // procfs.Proc is not available on windows and expected to fail. + pid := os.Getpid() + proc, err := procfs.NewProc(pid) + if err == nil { + pmv.proc = &proc + } + + return pmv } // StartCollection starts a ticker'd goroutine that will update the PMV measurements every 5 seconds func (pmv *ProcessMetricsViews) StartCollection() { - ticker := time.NewTicker(5 * time.Second) go func() { + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() for { select { case <-ticker.C: @@ -105,12 +116,12 @@ func (pmv *ProcessMetricsViews) StartCollection() { }() } -// Views returns the views internal to the PMV +// Views returns the views internal to the PMV. func (pmv *ProcessMetricsViews) Views() []*view.View { return pmv.views } -// StopCollection stops the collection of the process metric information +// StopCollection stops the collection of the process metric information. func (pmv *ProcessMetricsViews) StopCollection() { close(pmv.done) } @@ -122,10 +133,8 @@ func (pmv *ProcessMetricsViews) updateViews() { stats.Record(context.Background(), mRuntimeTotalAllocMem.M(int64(ms.TotalAlloc))) stats.Record(context.Background(), mRuntimeSysMem.M(int64(ms.Sys))) - pid := os.Getpid() - proc, err := procfs.NewProc(pid) - if err == nil { - if procStat, err := proc.Stat(); err == nil { + if pmv.proc != nil { + if procStat, err := pmv.proc.Stat(); err == nil { stats.Record(context.Background(), mCPUSeconds.M(int64(procStat.CPUTime()))) } } diff --git a/internal/collector/telemetry/process_telemetry_test.go b/internal/collector/telemetry/process_telemetry_test.go new file mode 100644 index 000000000000..ccf873f6a6f1 --- /dev/null +++ b/internal/collector/telemetry/process_telemetry_test.go @@ -0,0 +1,73 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package telemetry + +import ( + "runtime" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opencensus.io/stats/view" +) + +func TestProcessTelemetry(t *testing.T) { + const ballastSizeBytes uint64 = 0 + + pmv := NewProcessMetricsViews(ballastSizeBytes) + assert.NotNil(t, pmv) + + expectedViews := []string{ + // Changing a metric name is a breaking change. + // Adding new metrics is ok as long it follows the conventions described at + // https://pkg.go.dev/go.opentelemetry.io/collector/obsreport?tab=doc#hdr-Naming_Convention_for_New_Metrics + "process/runtime/heap_alloc_bytes", + "process/runtime/total_alloc_bytes", + "process/runtime/total_sys_memory_bytes", + "process/cpu_seconds", + } + processViews := pmv.Views() + assert.Len(t, processViews, len(expectedViews)) + + require.NoError(t, view.Register(processViews...)) + defer view.Unregister(processViews...) + + // Check that the views are actually filled. + pmv.updateViews() + <-time.After(200 * time.Millisecond) + + for _, viewName := range expectedViews { + if runtime.GOOS == "windows" && viewName == "process/cpu_seconds" { + continue + } + + rows, err := view.RetrieveData(viewName) + require.NoError(t, err, viewName) + + require.Len(t, rows, 1, viewName) + row := rows[0] + assert.Len(t, row.Tags, 0) + + lastValue := row.Data.(*view.LastValueData) + if viewName == "process/cpu_seconds" { + // This likely will still be zero when running the test. + assert.True(t, lastValue.Value >= 0, viewName) + continue + } + + assert.True(t, lastValue.Value > 0, viewName) + } +}