Azure Monitor: fix metric timespan to restore Storage Account PT1H me…

…trics (#40367) (#40413) Move the timespan logic into a dedicated `buildTimespan()` function with a test for each supported use case. Some Azure services have longer latency between service usage and metric availability. For example, the Storage Account capacity metrics (Blob capacity, etc.) have a PT1H time grain and become available after one hour. Service X also has PT1H metrics, however become available after a few minutes. This PR restores the core of the [older timespan logic](https://github.com/elastic/beats/blob/d3facc808d2ba293a42b2ad3fc8e21b66c5f2a7f/x-pack/metricbeat/module/azure/client.go#L110-L116) the Azure Monitor metricset was using before the regression introduced by the PR #36823. However, the `buildTimespan()` does not restore the `interval * (-2)` part because doubling the interval causes duplicates. (cherry picked from commit 5fccb0d) Co-authored-by: Maurizio Branca <[email protected]>
elastic · Aug 1, 2024 · 640f2de · 640f2de
1 parent c181443
commit 640f2de
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 10 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -93,6 +93,10 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
 *Metricbeat*
 
 - Fix statistic methods for metrics collected for SQS. {pull}40207[40207]
+- Add GCP 'instance_id' resource label in ECS cloud fields. {issue}40033[40033] {pull}40062[40062]
+- Fix missing metrics from CloudWatch when include_linked_accounts set to false. {issue}40071[40071] {pull}40135[40135]
+- Update beat module with apm-server monitoring metrics fields {pull}40127[40127]
+- Fix Azure Monitor metric timespan to restore Storage Account PT1H metrics {issue}40376[40376] {pull}40367[40367]
 
 *Osquerybeat*
 

diff --git a/x-pack/metricbeat/module/azure/azure.go b/x-pack/metricbeat/module/azure/azure.go
@@ -102,7 +102,6 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error {
 	//
 	// See "Round outer limits" and "Round inner limits" tests in
 	// the metric_registry_test.go for more information.
-	//referenceTime := time.Now().UTC().Round(time.Second)
 	referenceTime := time.Now().UTC()
 
 	// Initialize cloud resources and monitor metrics

diff --git a/x-pack/metricbeat/module/azure/client.go b/x-pack/metricbeat/module/azure/client.go
@@ -9,9 +9,10 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/monitor/armmonitor"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/monitor/armmonitor"
+
 	"github.com/elastic/beats/v7/metricbeat/mb"
 	"github.com/elastic/elastic-agent-libs/logp"
 )
@@ -119,19 +120,89 @@ func (client *Client) InitResources(fn mapResourceMetrics) error {
 	return nil
 }
 
+// buildTimespan returns the timespan for the metric values given the reference time,
+// time grain and collection period.
+//
+// (1) When the collection period is greater than the time grain, the timespan
+// will be:
+//
+// |                                            time grain
+// │                                          │◀──(PT1M)──▶ │
+// │                                                        │
+// ├──────────────────────────────────────────┼─────────────┼─────────────
+// │                                                        │
+// │                       timespan           │             │
+// |◀───────────────────────(5min)─────────────────────────▶│
+// │                                          │             │
+// |                        period                          │
+// │◀───────────────────────(5min)────────────┼────────────▶│
+// │                                                        │
+// │                                          │             │
+// |                                                        │
+// |                                                       Now
+// |                                                        │
+//
+// In this case, the API will return five metric values, because
+// the time grain is 1 minute and the timespan is 5 minutes.
+//
+// (2) When the collection period is equal to the time grain,
+// the timespan will be:
+//
+// |
+// │                       time grain                       │
+// |◀───────────────────────(5min)─────────────────────────▶│
+// │                                                        │
+// ├────────────────────────────────────────────────────────┼─────────────
+// │                                                        │
+// │                       timespan                         │
+// |◀───────────────────────(5min)─────────────────────────▶│
+// │                                                        │
+// |                        period                          │
+// │◀───────────────────────(5min)─────────────────────────▶│
+// │                                                        │
+// │                                                        │
+// |                                                        │
+// |                                                       Now
+// |                                                        │
+//
+// In this case, the API will return one metric value.
+//
+// (3) When the collection period is less than the time grain,
+// the timespan will be:
+//
+// |                                              period
+// │                                          │◀──(5min)──▶ │
+// │                                                        │
+// ├──────────────────────────────────────────┼─────────────┼─────────────
+// │                                                        │
+// │                       timespan           │             │
+// |◀───────────────────────(60min)────────────────────────▶│
+// │                                          │             │
+// |                      time grain                        │
+// │◀───────────────────────(PT1H)────────────┼────────────▶│
+// │                                                        │
+// │                                          │             │
+// |                                                       Now
+// |                                                        │
+// |
+//
+// In this case, the API will return one metric value.
+func buildTimespan(referenceTime time.Time, timeGrain string, collectionPeriod time.Duration) string {
+	timespanDuration := max(asDuration(timeGrain), collectionPeriod)
+
+	endTime := referenceTime
+	startTime := endTime.Add(timespanDuration * -1)
+
+	return fmt.Sprintf("%s/%s", startTime.Format(time.RFC3339), endTime.Format(time.RFC3339))
+}
+
 // GetMetricValues returns the metric values for the given cloud resources.
 func (client *Client) GetMetricValues(referenceTime time.Time, metrics []Metric, reporter mb.ReporterV2) []Metric {
 	var result []Metric
 
-	// Same end time for all metrics in the same batch.
-	interval := client.Config.Period
-
-	// Fetch in the range [{-2 x INTERVAL},{-1 x INTERVAL}) with a delay of {INTERVAL}.
-	endTime := referenceTime.Add(interval * (-1))
-	startTime := endTime.Add(interval * (-1))
-	timespan := fmt.Sprintf("%s/%s", startTime.Format(time.RFC3339), endTime.Format(time.RFC3339))
-
 	for _, metric := range metrics {
+		timespan := buildTimespan(referenceTime, metric.TimeGrain, client.Config.Period)
+
 		//
 		// Before fetching the metric values, check if the metric
 		// has been collected within the time grain.

diff --git a/x-pack/metricbeat/module/azure/client_test.go b/x-pack/metricbeat/module/azure/client_test.go
@@ -269,3 +269,56 @@ func TestGetMetricValues(t *testing.T) {
 		m.AssertExpectations(t)
 	})
 }
+
+func TestBuildBuildTimespan(t *testing.T) {
+	t.Run("Collection period greater than the time grain (PT1M metric every 5 minutes)", func(t *testing.T) {
+		referenceTime, _ := time.Parse(time.RFC3339, "2024-07-30T18:56:00Z")
+		timeGain := "PT1M"
+		collectionPeriod := 5 * time.Minute
+
+		timespan := buildTimespan(referenceTime, timeGain, collectionPeriod)
+
+		assert.Equal(t, "2024-07-30T18:51:00Z/2024-07-30T18:56:00Z", timespan)
+	})
+
+	t.Run("Collection period equal to time grain (PT1M metric every 1 minutes)", func(t *testing.T) {
+		referenceTime, _ := time.Parse(time.RFC3339, "2024-07-30T18:56:00Z")
+		timeGain := "PT1M"
+		collectionPeriod := 1 * time.Minute
+
+		timespan := buildTimespan(referenceTime, timeGain, collectionPeriod)
+
+		assert.Equal(t, "2024-07-30T18:55:00Z/2024-07-30T18:56:00Z", timespan)
+	})
+
+	t.Run("Collection period equal to time grain (PT5M metric every 5 minutes)", func(t *testing.T) {
+		referenceTime, _ := time.Parse(time.RFC3339, "2024-07-30T18:56:00Z")
+		timeGain := "PT5M"
+		collectionPeriod := 5 * time.Minute
+
+		timespan := buildTimespan(referenceTime, timeGain, collectionPeriod)
+
+		assert.Equal(t, "2024-07-30T18:51:00Z/2024-07-30T18:56:00Z", timespan)
+	})
+
+	t.Run("Collection period equal to time grain (PT1H metric every 60 minutes)", func(t *testing.T) {
+		referenceTime, _ := time.Parse(time.RFC3339, "2024-07-30T18:56:00Z")
+		timeGain := "PT1H"
+		collectionPeriod := 60 * time.Minute
+
+		timespan := buildTimespan(referenceTime, timeGain, collectionPeriod)
+
+		assert.Equal(t, "2024-07-30T17:56:00Z/2024-07-30T18:56:00Z", timespan)
+	})
+
+	t.Run("Collection period is less that time grain (PT1H metric every 5 minutes)", func(t *testing.T) {
+		referenceTime, _ := time.Parse(time.RFC3339, "2024-07-30T18:56:00Z")
+		timeGain := "PT1H"
+		collectionPeriod := 5 * time.Minute
+
+		timespan := buildTimespan(referenceTime, timeGain, collectionPeriod)
+
+		assert.Equal(t, "2024-07-30T17:56:00Z/2024-07-30T18:56:00Z", timespan)
+	})
+
+}