Skip to content

Commit

Permalink
[Metricbeat][Kibana] Apply backoff when errored at getting usage stats (
Browse files Browse the repository at this point in the history
#20772)

Co-authored-by: Shaunak Kashyap <ycombinator@gmail.com>
  • Loading branch information
afharo and ycombinator committed Sep 18, 2020
1 parent 9e4ba21 commit 5332b2d
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Expand Up @@ -339,6 +339,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix SQL module mapping NULL values as string {pull}18955[18955] {issue}18898[18898
- Fix ec2 disk and network metrics to use Sum statistic method. {pull}20680[20680]
- Fill cloud.account.name with accountID if account alias doesn't exist. {pull}20736[20736]
- The Kibana collector applies backoff when errored at getting usage stats {pull}20772[20772]
- Update fields.yml in the azure module, missing metrics field. {pull}20918[20918]
- The `elasticsearch/index` metricset only requests wildcard expansion for hidden indices if the monitored Elasticsearch cluster supports it. {pull}20938[20938]
- Disable Kafka metricsets based on Jolokia by default. They require a different configuration. {pull}20989[20989]
Expand Down
14 changes: 10 additions & 4 deletions metricbeat/module/kibana/stats/stats.go
Expand Up @@ -38,9 +38,10 @@ func init() {
}

const (
statsPath = "api/stats"
settingsPath = "api/settings"
usageCollectionPeriod = 24 * time.Hour
statsPath = "api/stats"
settingsPath = "api/settings"
usageCollectionPeriod = 24 * time.Hour
usageCollectionBackoff = 1 * time.Hour
)

var (
Expand All @@ -57,6 +58,7 @@ type MetricSet struct {
statsHTTP *helper.HTTP
settingsHTTP *helper.HTTP
usageLastCollectedOn time.Time
usageNextCollectOn time.Time
isUsageExcludable bool
}

Expand Down Expand Up @@ -165,6 +167,10 @@ func (m *MetricSet) fetchStats(r mb.ReporterV2, now time.Time) error {

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
}

Expand Down Expand Up @@ -215,5 +221,5 @@ func (m *MetricSet) calculateIntervalMs() int64 {
}

func (m *MetricSet) shouldCollectUsage(now time.Time) bool {
return now.Sub(m.usageLastCollectedOn) > usageCollectionPeriod
return now.Sub(m.usageLastCollectedOn) > usageCollectionPeriod && now.Sub(m.usageNextCollectOn) > 0
}
44 changes: 41 additions & 3 deletions metricbeat/module/kibana/stats/stats_test.go
Expand Up @@ -23,6 +23,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/stretchr/testify/require"

Expand All @@ -48,12 +49,12 @@ func TestFetchUsage(t *testing.T) {
w.WriteHeader(503)

case 1: // second call
// Make sure exclude_usage is still false since first call failed
require.Equal(t, "false", excludeUsage)
// 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)
w.WriteHeader(200)

case 2: // third call
// Make sure exclude_usage is now true since second call succeeded
// Make sure exclude_usage is still true
require.Equal(t, "true", excludeUsage)
w.WriteHeader(200)
}
Expand All @@ -76,3 +77,40 @@ func TestFetchUsage(t *testing.T) {
// Third fetch
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,
}

actualResult := m.shouldCollectUsage(now)
require.Equal(t, test.expectedResult, actualResult)
})
}
}

0 comments on commit 5332b2d

Please sign in to comment.