diff --git a/.chloggen/fix_46977.yaml b/.chloggen/fix_46977.yaml new file mode 100644 index 0000000000000..a7789eaf8d145 --- /dev/null +++ b/.chloggen/fix_46977.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# 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. receiver/filelog) +component: receiver/vcenter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fixes a nil pointer dereference panic in recordVMStats when scraping metrics from VMs with missing performance counters + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [46977] + +# (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: + +# 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. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/receiver/vcenterreceiver/metrics.go b/receiver/vcenterreceiver/metrics.go index 78b7f5f9eeb11..e1d80c83edcc9 100644 --- a/receiver/vcenterreceiver/metrics.go +++ b/receiver/vcenterreceiver/metrics.go @@ -258,6 +258,9 @@ func (v *vcenterMetricScraper) recordVMStats( vm *mo.VirtualMachine, hs *mo.HostSystem, ) { + if vm.Summary.Storage == nil || vm.Config == nil || hs == nil || hs.Summary.Hardware == nil { + return + } diskUsed := vm.Summary.Storage.Committed diskFree := vm.Summary.Storage.Uncommitted diff --git a/receiver/vcenterreceiver/metrics_test.go b/receiver/vcenterreceiver/metrics_test.go new file mode 100644 index 0000000000000..b48e170bf8ce5 --- /dev/null +++ b/receiver/vcenterreceiver/metrics_test.go @@ -0,0 +1,108 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package vcenterreceiver // import github.com/open-telemetry/opentelemetry-collector-contrib/receiver/vcenterreceiver + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/vmware/govmomi/vim25/mo" + "github.com/vmware/govmomi/vim25/types" + "go.opentelemetry.io/collector/pdata/pcommon" + "go.opentelemetry.io/collector/receiver/receivertest" + + "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/vcenterreceiver/internal/metadata" +) + +func TestRecordVMStats_IncompleteDataDoesNotPanic(t *testing.T) { + scraper := &vcenterMetricScraper{ + mb: metadata.NewMetricsBuilder(metadata.DefaultMetricsBuilderConfig(), receivertest.NewNopSettings(metadata.Type)), + } + ts := pcommon.NewTimestampFromTime(time.Now()) + + validVM := &mo.VirtualMachine{ + Config: &types.VirtualMachineConfigInfo{}, + Summary: types.VirtualMachineSummary{ + Storage: &types.VirtualMachineStorageSummary{}, + }, + } + validHost := &mo.HostSystem{ + Summary: types.HostListSummary{ + Hardware: &types.HostHardwareSummary{}, + }, + } + + testCases := []struct { + name string + vm *mo.VirtualMachine + hs *mo.HostSystem + }{ + { + name: "nil vm config", + vm: &mo.VirtualMachine{ + Summary: types.VirtualMachineSummary{ + Storage: &types.VirtualMachineStorageSummary{}, + }, + }, + hs: validHost, + }, + { + name: "nil vm storage summary", + vm: &mo.VirtualMachine{ + Config: &types.VirtualMachineConfigInfo{}, + }, + hs: validHost, + }, + { + name: "nil host", + vm: validVM, + hs: nil, + }, + { + name: "nil host summary hardware", + vm: validVM, + hs: &mo.HostSystem{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.NotPanics(t, func() { + scraper.recordVMStats(ts, tc.vm, tc.hs) + }) + }) + } +} + +func TestBuildVMMetrics_IncompleteVMSkipsWithoutError(t *testing.T) { + scraper := &vcenterMetricScraper{ + scrapeData: &vcenterScrapeData{ + computesByRef: map[string]*mo.ComputeResource{ + "cr-1": {}, + }, + }, + } + + vm := &mo.VirtualMachine{ + Runtime: types.VirtualMachineRuntimeInfo{PowerState: types.VirtualMachinePowerStatePoweredOff}, + } + vmRefToComputeRef := map[string]*types.ManagedObjectReference{ + "": {Type: "ComputeResource", Value: "cr-1"}, + } + + crRef, groupInfo, err := scraper.buildVMMetrics( + pcommon.NewTimestampFromTime(time.Now()), + &mo.Datacenter{}, + vm, + vmRefToComputeRef, + ) + + require.NoError(t, err) + require.NotNil(t, crRef) + require.NotNil(t, groupInfo) + require.Equal(t, int64(1), groupInfo.poweredOff) + require.Equal(t, int64(0), groupInfo.poweredOn) + require.Equal(t, int64(0), groupInfo.templates) +} diff --git a/receiver/vcenterreceiver/processors.go b/receiver/vcenterreceiver/processors.go index aed66088a0da7..9e048c515ebd0 100644 --- a/receiver/vcenterreceiver/processors.go +++ b/receiver/vcenterreceiver/processors.go @@ -289,6 +289,25 @@ func (v *vcenterMetricScraper) buildVMMetrics( return crRef, groupInfo, fmt.Errorf("no collected ComputeResource for VM [%s]'s ComputeResource ref: %s", vm.Name, crRef) } + groupInfo = &vmGroupInfo{poweredOff: 0, poweredOn: 0, suspended: 0, templates: 0} + if vm.Config != nil && vm.Config.Template { + groupInfo.templates++ + } else { + switch vm.Runtime.PowerState { + case "poweredOff": + groupInfo.poweredOff++ + case "poweredOn": + groupInfo.poweredOn++ + default: + groupInfo.suspended++ + } + } + + // Powered-off/suspended/template VMs can have incomplete data from vSphere. + if vm.Config == nil || vm.Summary.Storage == nil { + return crRef, groupInfo, nil + } + // Get related VM host info hsRef := vm.Summary.Runtime.Host if hsRef == nil { @@ -307,20 +326,6 @@ func (v *vcenterMetricScraper) buildVMMetrics( rp = v.scrapeData.rPoolsByRef[rpRef.Value] } - groupInfo = &vmGroupInfo{poweredOff: 0, poweredOn: 0, suspended: 0, templates: 0} - if vm.Config != nil && vm.Config.Template { - groupInfo.templates++ - } else { - switch vm.Runtime.PowerState { - case "poweredOff": - groupInfo.poweredOff++ - case "poweredOn": - groupInfo.poweredOn++ - default: - groupInfo.suspended++ - } - } - // Create VM resource builder rb, err := v.createVMResourceBuilder(dc, cr, hs, rp, vm) if err != nil {