fix: Safeguard closing nil channels in azure metrics module (#50498)

* fix: Safeguard closing nil channels in azure metrics module

* add changelog

Author: MichaelKatsoulis

changelog/fragments/1777989631-fix-azure-batch-client-panic-on-empty-resources.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: Fix panic in azure module when all configured resources match no Azure resources
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: metricbeat
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+pr: https://github.com/elastic/beats/pull/50498
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234

x-pack/metricbeat/module/azure/client_batch.go

@@ -137,13 +137,19 @@ func (client *BatchClient) InitResources(fn concurrentMapResourceMetrics) error
 		fn(client, resourceList, resourceConfig, &wg)
 		client.Log.Infof("Finished collection with %d metric definitions", len(resourceList))
 	}
-	go func() {
-		wg.Wait() // Wait for all the resource collection goroutines to finish
-		// Once all the goroutines are done, close the channels
-		client.Log.Debug("All collections finished. Closing channels ")
-		close(client.ResourceConfigurations.MetricDefinitionsChan)
-		close(client.ResourceConfigurations.ErrorChan)
-	}()
+	// Only launch the closer goroutine if at least one resource config produced
+	// results and created the channels. When every config returns an empty
+	// list the channels stay nil and the consumer will surface the
+	// "no resources were found" error via its own nil-check.
+	if client.ResourceConfigurations.MetricDefinitionsChan != nil && client.ResourceConfigurations.ErrorChan != nil {
+		go func() {
+			wg.Wait() // Wait for all the resource collection goroutines to finish
+			// Once all the goroutines are done, close the channels
+			client.Log.Debug("All collections finished. Closing channels ")
+			close(client.ResourceConfigurations.MetricDefinitionsChan)
+			close(client.ResourceConfigurations.ErrorChan)
+		}()
+	}
 	return nil
 }
 

x-pack/metricbeat/module/azure/client_batch_test.go

@@ -47,6 +47,21 @@ func TestInitResourcesForBatch(t *testing.T) {
 		assert.Error(t, err, "failed to retrieve resources: invalid resource query")
 		m.AssertExpectations(t)
 	})
+	t.Run("does not panic when all resource configs return an empty list", func(t *testing.T) {
+		client := NewMockBatchClient(logger)
+		client.Config = resourceQueryConfig
+		m := &MockService{}
+		// empty list + nil error: the "no resources of this type exist" path
+		m.On("GetResourceDefinitions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*armresources.GenericResourceExpanded{}, nil)
+		client.AzureMonitorService = m
+
+		err := client.InitResources(mockConcurrentMapResourceMetrics)
+		require.NoError(t, err)
+
+		// The closer goroutine launched by InitResources runs asynchronously and
+		// will panic on close(nil) if the channels were never initialized.
+		time.Sleep(100 * time.Millisecond)
+	})
 }
 
 func TestGetMetricsInBatch(t *testing.T) {
@@ -99,7 +114,7 @@ func TestGetMetricsInBatch(t *testing.T) {
 		mr := MockReporterV2{}
 		mr.On("Error", mock.Anything).Return(true)
 		results := client.GetMetricsInBatch(groupedMetrics, referenceTime, &mr)
-		assert.Equal(t, len(results), 0)
+		assert.Empty(t, results, "expected no metric values when QueryResources returns an error")
 		m.AssertExpectations(t)
 	})
 
@@ -169,12 +184,12 @@ func TestGetMetricsInBatch(t *testing.T) {
 		mr := MockReporterV2{}
 
 		metricValues := client.GetMetricsInBatch(groupedMetrics, referenceTime, &mr)
-		require.Equal(t, len(metricValues), 1)
-		require.Equal(t, len(metricValues[0].Values), 1)
+		require.Len(t, metricValues, 1, "expected exactly one metric value group")
+		require.Len(t, metricValues[0].Values, 1, "expected exactly one value in the metric group")
 
-		assert.Equal(t, *metricValues[0].Values[0].avg, 1.0)
-		assert.Equal(t, *metricValues[0].Values[0].max, 2.0)
-		assert.Equal(t, *metricValues[0].Values[0].min, 3.0)
+		assert.InDelta(t, 1.0, *metricValues[0].Values[0].avg, 0.0001)
+		assert.InDelta(t, 2.0, *metricValues[0].Values[0].max, 0.0001)
+		assert.InDelta(t, 3.0, *metricValues[0].Values[0].min, 0.0001)
 
 		m.AssertExpectations(t)
 	})