Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry-pick to 7.x: [Metricbeat/Kibana/stats] Enforce exclude_usage=true (#22732) #22755

Merged
merged 2 commits into from
Nov 26, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the changelog.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thank you for noticing! Fixed!

- `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*

Expand Down
45 changes: 11 additions & 34 deletions metricbeat/module/kibana/stats/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package stats

import (
"fmt"
"strconv"
"strings"
"time"

Expand All @@ -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 (
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
65 changes: 24 additions & 41 deletions metricbeat/module/kibana/stats/stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}

Expand All @@ -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)
}