[Metricbeat/Kibana/stats] Enforce exclude_usage=true (elastic#22732)

CHANGELOG.next.asciidoc

@@ -601,6 +601,19 @@ same journal. {pull}18467[18467]
 - Add awsfargate module task_stats metricset to monitor AWS ECS Fargate. {pull}22034[22034]
 - Add connection and route metricsets for nats metricbeat module to collect metrics per connection/route. {pull}22445[22445]
 - Add unit file states to system/service {pull}22557[22557]
+- Add io.ops in fields exported by system.diskio. {pull}22066[22066]
+- `kibana` module: `stats` metricset no-longer collects usage-related data. {pull}22732[22732]
+
+*Packetbeat*
+
+- Add an example to packetbeat.yml of using the `forwarded` tag to disable
+  `host` metadata fields when processing network data from network tap or mirror
+  port. {pull}19209[19209]
+- Add ECS fields for x509 certs, event categorization, and related IP info. {pull}19167[19167]
+- Add 100-continue support {issue}15830[15830] {pull}19349[19349]
+- Add initial SIP protocol support {pull}21221[21221]
+- Add support for overriding the published index on a per-protocol/flow basis. {pull}22134[22134]
+- Change build process for x-pack distribution {pull}21979[21979]
 
 *Packetbeat*
 

metricbeat/module/kibana/stats/stats.go

@@ -19,7 +19,6 @@ package stats
 
 import (
 	"fmt"
-	"strconv"
 	"strings"
 	"time"
 
@@ -38,10 +37,8 @@ func init() {
 }
 
 const (
-	statsPath              = "api/stats"
-	settingsPath           = "api/settings"
-	usageCollectionPeriod  = 24 * time.Hour
-	usageCollectionBackoff = 1 * time.Hour
+	statsPath    = "api/stats"
+	settingsPath = "api/settings"
 )
 
 var (
@@ -55,11 +52,9 @@ var (
 // MetricSet type defines all fields of the MetricSet
 type MetricSet struct {
 	*kibana.MetricSet
-	statsHTTP            *helper.HTTP
-	settingsHTTP         *helper.HTTP
-	usageLastCollectedOn time.Time
-	usageNextCollectOn   time.Time
-	isUsageExcludable    bool
+	statsHTTP         *helper.HTTP
+	settingsHTTP      *helper.HTTP
+	isUsageExcludable bool
 }
 
 // New create a new instance of the MetricSet
@@ -157,31 +152,17 @@ func (m *MetricSet) fetchStats(r mb.ReporterV2, now time.Time) error {
 	var content []byte
 	var err error
 
-	// Collect usage stats only once every usageCollectionPeriod
+	// Add exclude_usage=true if the Kibana Version supports it
 	if m.isUsageExcludable {
 		origURI := m.statsHTTP.GetURI()
 		defer m.statsHTTP.SetURI(origURI)
 
-		shouldCollectUsage := m.shouldCollectUsage(now)
-		m.statsHTTP.SetURI(origURI + "&exclude_usage=" + strconv.FormatBool(!shouldCollectUsage))
-
-		content, err = m.statsHTTP.FetchContent()
-		if err != nil {
-			if shouldCollectUsage {
-				// When errored in collecting the usage stats it may be counterproductive to try again on the next poll, try to collect the stats again after usageCollectionBackoff
-				m.usageNextCollectOn = now.Add(usageCollectionBackoff)
-			}
-			return err
-		}
+		m.statsHTTP.SetURI(origURI + "&exclude_usage=true")
+	}
 
-		if shouldCollectUsage {
-			m.usageLastCollectedOn = now
-		}
-	} else {
-		content, err = m.statsHTTP.FetchContent()
-		if err != nil {
-			return err
-		}
+	content, err = m.statsHTTP.FetchContent()
+	if err != nil {
+		return err
 	}
 
 	if m.XPackEnabled {
@@ -219,7 +200,3 @@ func (m *MetricSet) fetchSettings(r mb.ReporterV2, now time.Time) {
 func (m *MetricSet) calculateIntervalMs() int64 {
 	return m.Module().Config().Period.Nanoseconds() / 1000 / 1000
 }
-
-func (m *MetricSet) shouldCollectUsage(now time.Time) bool {
-	return now.Sub(m.usageLastCollectedOn) > usageCollectionPeriod && now.Sub(m.usageNextCollectOn) > 0
-}

metricbeat/module/kibana/stats/stats_test.go

@@ -23,15 +23,14 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 
 	mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing"
 	"github.com/elastic/beats/v7/metricbeat/module/kibana/mtest"
 )
 
-func TestFetchUsage(t *testing.T) {
+func TestFetchExcludeUsage(t *testing.T) {
 	// Spin up mock Kibana server
 	numStatsRequests := 0
 	kib := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -45,17 +44,15 @@ func TestFetchUsage(t *testing.T) {
 			// Make GET /api/stats return 503 for first call, 200 for subsequent calls
 			switch numStatsRequests {
 			case 0: // first call
-				require.Equal(t, "false", excludeUsage)
+				require.Equal(t, "true", excludeUsage) // exclude_usage is always true
 				w.WriteHeader(503)
 
 			case 1: // second call
-				// Make sure exclude_usage is true since first call failed and it should not try again until usageCollectionBackoff time has passed
-				require.Equal(t, "true", excludeUsage)
+				require.Equal(t, "true", excludeUsage) // exclude_usage is always true
 				w.WriteHeader(200)
 
 			case 2: // third call
-				// Make sure exclude_usage is still true
-				require.Equal(t, "true", excludeUsage)
+				require.Equal(t, "true", excludeUsage) // exclude_usage is always true
 				w.WriteHeader(200)
 			}
 
@@ -78,39 +75,25 @@ func TestFetchUsage(t *testing.T) {
 	mbtest.ReportingFetchV2Error(f)
 }
 
-func TestShouldCollectUsage(t *testing.T) {
-	now := time.Now()
-
-	cases := map[string]struct {
-		usageLastCollectedOn time.Time
-		usageNextCollectOn   time.Time
-		expectedResult       bool
-	}{
-		"within_usage_collection_period": {
-			usageLastCollectedOn: now.Add(-1 * usageCollectionPeriod),
-			expectedResult:       false,
-		},
-		"after_usage_collection_period_but_before_next_scheduled_collection": {
-			usageLastCollectedOn: now.Add(-2 * usageCollectionPeriod),
-			usageNextCollectOn:   now.Add(3 * time.Hour),
-			expectedResult:       false,
-		},
-		"after_usage_collection_period_and_after_next_scheduled_collection": {
-			usageLastCollectedOn: now.Add(-2 * usageCollectionPeriod),
-			usageNextCollectOn:   now.Add(-1 * time.Hour),
-			expectedResult:       true,
-		},
-	}
-
-	for name, test := range cases {
-		t.Run(name, func(t *testing.T) {
-			m := MetricSet{
-				usageLastCollectedOn: test.usageLastCollectedOn,
-				usageNextCollectOn:   test.usageNextCollectOn,
-			}
+func TestFetchNoExcludeUsage(t *testing.T) {
+	// Spin up mock Kibana server
+	kib := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/api/status":
+			w.Write([]byte("{ \"version\": { \"number\": \"7.0.0\" }}")) // v7.0.0 does not support exclude_usage and should not be sent
 
-			actualResult := m.shouldCollectUsage(now)
-			require.Equal(t, test.expectedResult, actualResult)
-		})
-	}
+		case "/api/stats":
+			excludeUsage := r.FormValue("exclude_usage")
+			require.Empty(t, excludeUsage) // exclude_usage should not be provided
+			w.WriteHeader(200)
+		}
+	}))
+	defer kib.Close()
+
+	config := mtest.GetConfig("stats", kib.URL, true)
+
+	f := mbtest.NewReportingMetricSetV2Error(t, config)
+
+	// First fetch
+	mbtest.ReportingFetchV2Error(f)
 }