From 06ff260b962d4490e0dea92cc9eb8c0ce09ee64b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Ferna=CC=81ndez=20Haro?= Date: Tue, 24 Nov 2020 12:27:19 +0100 Subject: [PATCH 1/3] [Metricbeat/Kibana/stats] `exclude_usage=true` by default --- metricbeat/module/kibana/stats/stats.go | 43 +++---------- metricbeat/module/kibana/stats/stats_test.go | 65 ++++++++------------ 2 files changed, 34 insertions(+), 74 deletions(-) diff --git a/metricbeat/module/kibana/stats/stats.go b/metricbeat/module/kibana/stats/stats.go index a6e19d50f42c..836ac177c0ec 100644 --- a/metricbeat/module/kibana/stats/stats.go +++ b/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 @@ -162,26 +157,12 @@ func (m *MetricSet) fetchStats(r mb.ReporterV2, now time.Time) error { 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 -} diff --git a/metricbeat/module/kibana/stats/stats_test.go b/metricbeat/module/kibana/stats/stats_test.go index 56cbfc17e1f2..a7b76603352e 100644 --- a/metricbeat/module/kibana/stats/stats_test.go +++ b/metricbeat/module/kibana/stats/stats_test.go @@ -23,7 +23,6 @@ import ( "net/http" "net/http/httptest" "testing" - "time" "github.com/stretchr/testify/require" @@ -31,7 +30,7 @@ import ( "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) } From e49f7ec4b5cf395ab1fc02757d6f42b8a0ea34b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Ferna=CC=81ndez=20Haro?= Date: Tue, 24 Nov 2020 12:35:36 +0100 Subject: [PATCH 2/3] Add CHANGELOG --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 5b8d51369794..e22b0300640c 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -849,6 +849,7 @@ same journal. {pull}18467[18467] - 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* From 6a17c2ba42fba6f1fbf7eefe5ca0f2271402502f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Ferna=CC=81ndez=20Haro?= Date: Wed, 25 Nov 2020 10:20:27 +0100 Subject: [PATCH 3/3] Fix comment --- metricbeat/module/kibana/stats/stats.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metricbeat/module/kibana/stats/stats.go b/metricbeat/module/kibana/stats/stats.go index 836ac177c0ec..0335e814fd4a 100644 --- a/metricbeat/module/kibana/stats/stats.go +++ b/metricbeat/module/kibana/stats/stats.go @@ -152,7 +152,7 @@ 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)