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.

Sign up for GitHub

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

Jump to bottom

[Metricbeat] Kibana/stats set exclude_usage=true in the init phase #29911

Closed
wants to merge 3 commits into from
Closed

[Metricbeat] Kibana/stats set exclude_usage=true in the init phase #29911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1a8f6d
[Metricbeat] Kibana/stats set `exclude_usage=true` in the `init` phase
afharo Jan 19, 2022
bd1c93f
Merge branch 'master' of github.com:elastic/beats into metricbeat/kib…
afharo Jan 20, 2022
4e54607
Changelog
afharo Jan 20, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Extract correct index property in kibana.stats metricset {pull}29622[29622]
- Fixed bug with `elasticsearch/cluster_stats` metricset not recording license expiration date correctly. {pull}29711[29711]
- Fixed GCP GKE Overview dashboard {pull}29913[29913]
- Set the final URL in the `init` phase in the Kibana-stats module {pull}29911[29911]

*Packetbeat*

Expand Down
24 changes: 24 additions & 0 deletions metricbeat/module/kibana/kibana_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,27 @@ func TestIsStatsAPIAvailable(t *testing.T) {
require.Equal(t, test.expected, actual)
}
}

func TestIsUsageExcludable(t *testing.T) {
tests := []struct {
input string
expected bool
}{
{"6.3.1", false},
{"6.4.0", false},
{"6.5.0", false},
{"6.7.2", true},
{"6.8.16", true},
{"7.0.0-alpha1", false},
{"7.0.0", false},
{"7.0.1", true},
{"7.0.2", true},
{"7.5.0", true},
{"7.16.2", true},
}

for _, test := range tests {
actual := kibana.IsUsageExcludable(common.MustNewVersion(test.input))
require.Equal(t, test.expected, actual)
}
}
18 changes: 7 additions & 11 deletions metricbeat/module/kibana/stats/stats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ var (
// MetricSet type defines all fields of the MetricSet
type MetricSet struct {
*kibana.MetricSet
statsHTTP *helper.HTTP
isUsageExcludable bool
statsHTTP *helper.HTTP
}

// New create a new instance of the MetricSet
Expand Down Expand Up @@ -95,8 +94,13 @@ func (m *MetricSet) init() error {
return fmt.Errorf(errorMsg, m.FullyQualifiedName(), kibana.StatsAPIAvailableVersion, kibanaVersion)
}

// Add exclude_usage=true if the Kibana Version supports it
if kibana.IsUsageExcludable(kibanaVersion) {
origURI := statsHTTP.GetURI()
statsHTTP.SetURI(origURI + "&exclude_usage=true")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll double check this change. The code in this part is definitely obscure but if the defer statement is deleted here, it might be the case that the uri will be &exclude_usage=true&exclude_usage=true&exclude_usage=true after 3 periods.

Copy link
Member Author

Choose a reason for hiding this comment

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

it makes perfect sense! Is there any documentation I can use to run this PR? Or any artifacts generated during CI that I can use to test?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope sorry, you'll have to do a go build and setup everything 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

oh wait you can use https://github.com/elastic/elastic-package to spin up the entire environment and then use go build with your code.

Copy link
Member Author

@afharo afharo Feb 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@sayden I spent a while trying to test it locally, but I'm on an M1 Mac now and there's a library that is no-longer compatible. I'd appreciate some help when you have some time 😇

Copy link
Contributor

Choose a reason for hiding this comment

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

I have just seen this. I'll do a try asap

}

m.statsHTTP = statsHTTP
m.isUsageExcludable = kibana.IsUsageExcludable(kibanaVersion)

return nil
}
Expand All @@ -105,14 +109,6 @@ func (m *MetricSet) fetchStats(r mb.ReporterV2) error {
var content []byte
var err error

// Add exclude_usage=true if the Kibana Version supports it
if m.isUsageExcludable {
origURI := m.statsHTTP.GetURI()
defer m.statsHTTP.SetURI(origURI)

m.statsHTTP.SetURI(origURI + "&exclude_usage=true")
}

content, err = m.statsHTTP.FetchContent()
if err != nil {
return err
Expand Down