[Metricbeat/Kibana/stats] exclude_usage=true by default

[afharo]

What does this PR do?

It adds changes to the Metricbeat's kibana module: it adds a new configuration flag stats.exclude_usage (default true) that is used in the metricset stats in the method shouldCollectUsage to decide whether it should collect usage (no matter the time).

The reason for this change is to disable the usage collection via the /api/stats endpoint but to still allow the collection of the usage via explicitly setting the flag to false if necessary.

Why is it important?

We are working towards decoupling Monitoring and Telemetry in Kibana. At the moment, Monitoring holds in the .monitoring-kibana-* indices the Kibana's Usage details only to be able to report them when sending the telemetry reports. This data is already sent to the remote telemetry cluster via Kibana's local collectors, so there's no need for this duplication in the efforts. Thanks to these changes, Monitoring will only maintain in its indices the data they really need for its own features.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

• Did I filled the default config in the right way?
• Not a Go developer myself. Did I mess anything up? 😇

How to test this PR locally

Setup Metricbeat with the kibana-xpack module enabled and notice it never asks for the /api/stats?extended&legacy&exclude_usage=false request (the last query is always true by default).

Related issues

• Closes [Metricbeat][Kibana][stats] Stop collecting usage #22651
• Relates [Telemetry/Monitoring] Monitoring should stop collecting Kibana Usage in the .monitoring indices kibana#83521
• Relates [Meta][Telemetry] Monitoring decoupling kibana#83626

Use cases

Explained in the issue #22651

Screenshots

Logs

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Build Cause: [Pull request #22654 updated]

• Start Time: 2020-11-20T12:28:33.120+0000

• Duration: 62 min 28 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2218
Skipped	505
Total	2723

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	2218
Skipped	505
Total	2723

[andresrc]

cc @sayden for awareness given the ongoing changes

[elasticmachine]

Pinging @elastic/stack-monitoring (Stack monitoring)

[elasticmachine]

Pinging @elastic/integrations-services (Team:Services)

[afharo]

cc @chrisronline for awareness

[chrisronline]

@sayden @ycombinator This change relates a larger initiative we are undertaking to decouple telemetry and monitoring. For Kibana, we will no longer collect and index usage data, as we will rely entirely in local telemetry collection within Kibana instances (which is enabled by default). See elastic/kibana#83626 for more information

[ycombinator]

Hey @afharo, thanks for this PR.

I understand the reasons for this change and am generally ++ for it. However, I'd like to explore the possibility of not introducing a configuration setting to toggle this change. Instead, I'd like to explore if we could programmatically determine the right value for the exclude_usage flag based on some conditions.

With the change in this PR, the kibana/stats metricset will no longer be responsible for collecting usage stats from Kibana and indexing it into Stack Monitoring indices. And subsequently, Telemetry code in Kibana will no longer have access to these usage stats from the Stack Monitoring indices. So I imagine there is a parallel effort happening in Kibana to get the same usage stats into Telemetry some other way. Which version of Kibana is this parallel effort expected to land in? Maybe we could toggle the exclude_usage flag in the kibana/stats metricset based on this version (we do this sort of thing already in a couple of places:

beats/metricbeat/module/kibana/kibana.go

Lines 92 to 100 in 0d4a830

	// IsStatsAPIAvailable returns whether the stats API is available in the given version of Kibana
	func IsStatsAPIAvailable(currentKibanaVersion *common.Version) bool {
	return elastic.IsFeatureAvailable(currentKibanaVersion, StatsAPIAvailableVersion)
	}
	// IsSettingsAPIAvailable returns whether the settings API is available in the given version of Kibana
	func IsSettingsAPIAvailable(currentKibanaVersion *common.Version) bool {
	return elastic.IsFeatureAvailable(currentKibanaVersion, SettingsAPIAvailableVersion)
	}

). Essentially what we'll be saying is: starting from version X, Kibana knows how to collect usage stats on its own, so no need for the kibana/stats metricset to collect it.

[afharo]

Hey @ycombinator,

Thanks for coming back to me! These are very valid points! Our intention is to ship the parallel effort in Kibana 7.11 (almost there 😅).

I'd love to use your knowledge here to deep dive on this version-dependency: The way Telemetry will use the Monitoring data depends on the Monitoring cluster's version, but not the monitored production clusters. This means that:

• If the Monitoring cluster is 7.11 and all the production clusters are running any other version, the telemetry will be fine.
• If the Production cluster is 7.11, but the Monitoring cluster is not at least 7.11, the reported telemetry might be incomplete.

Do you know if Metricbeat is, somehow, aware of the version of the Monitoring cluster it is sending the stats to?

FYI: Telemetry will still be fine if Monitoring data contains Kibana Usage on 7.11. So we could take this into a longer effort and wait for Metricbeat to stop reporting this data on a future version (i.e.: Make exclude_usage=false by default for now and change it in a future release).

[ycombinator]

Ah, thanks for clarifying the monitoring vs. monitored cluster situation here. There's no way for Metricbeat to know which monitoring cluster it's sending the data to so we can't check it's version.

Given that, I think what you have in this PR is a good approach for now - we default to the current behavior but provide a flag to turn it off in a future release. One question then becomes: do we need documentation to tell users to explicitly set the setting to false if they are running 7.11 or greater in their monitoring cluster?

Alternatively, we could just turn it off completely in this PR - i.e. no setting to toggle it - but only merge it into master, i.e. we won't backport it to 7.x. This will mean that starting 8.0, the kibana/stats metricset in Metricbeat will never collect usage and versions before that (7.x) will always collect usage (which is the current behavior).

[afharo]

Given that, I think what you have in this PR is a good approach for now - we default to the current behavior but provide a flag to turn it off in a future release.

In this PR I actually did the opposite: disabled by default, but a flag to turn it back on if needed. But I'm happy to flip the default value if you think it's safer.

One question then becomes: do we need documentation to tell users to explicitly set the setting to false if they are running 7.11 or greater in their monitoring cluster?

Given that we are doing the opposite change in this PR (for now), I think we should tell our users to explicitly set false if their monitoring cluster is 7.10 or earlier to maintain backwards compatiblity.

Alternatively, we could just turn it off completely in this PR - i.e. no setting to toggle it - but only merge it into master, i.e. we won't backport it to 7.x. This will mean that starting 8.0, the kibana/stats metricset in Metricbeat will never collect usage and versions before that (7.x) will always collect usage (which is the current behavior).

I think, ultimately, we'll complete this breaking change and remove the option flag entirely. Whether to keep collecting it until 8.0 or not, I don't have a strong opinion. I'd like to know @chrisronline's and @kobelb's opinions on that.

[ycombinator]

So, maybe the naming of the setting is confusing me :)

Given that we are doing the opposite change in this PR (for now), I think we should tell our users to explicitly set false if their monitoring cluster is 7.10 or earlier to maintain backwards compatiblity.

I think it's a bit odd to ask users to take some action in a minor version to maintain backwards compatibility. I think backwards compatibility should be expected without any extra actions by the user between minor version upgrades.

So I think the default value of the setting should result in the behavior we have today (which is for the kibana/stats metricset to collect usage stats). That way, users of Metricbeat 7.11 (assuming this PR makes it to that release) should not need to do anything extra right after they upgrade just to maintain backwards compatibility. Then, we also add to documentation saying: if your monitoring cluster is >= 7.11, please set X setting to Y value for so and so reason. And from a user perspective there is a compelling reason: usage stats docs can be quite large so not collecting them via Metricbeat saves memory + size on the wire.

Then in 8.0 / master we can simply remove the logic from the kibana/stats metricset to collect usage stats as well as the setting to allow users to toggle this collection. This would be a breaking change, but that's okay since it would be across a major version boundary.

[afharo]

@ycombinator sounds like a very sensible plan to me! Thank you! I'll apply those changes to this PR.

[kobelb]

It's unclear to me why changing the behavior of Metricbeat to exclude the usage would be a breaking change from the end-users perspective. The output of Kibana's stats endpoint is indexed into the .monitoring-kibana-* and end-users shouldn't be querying these documents directly. Changing this behavior should only affect the telemetry data that is sent to Elastic itself, and we're tolerant of this change in a minor version.

[ycombinator]

That's a good distinction and clarification, @kobelb. From the POV of a maintainer of the kibana Metricbeat module, I've been considering end-users and internal (Elastic) users both as users of this module, in that they are both affected by what data this module collects.

Asking the kibana/stats Metricbeat module to stop collecting usage telemetry is definitely not a breaking change for end-users. And if internal users are tolerant to not having usage telemetry in case an end-user runs Metricbeat 7.11+ against a monitoring cluster that's < 7.11 (as @afharo explained in #22654 (comment)), then I'm ++ removing usage telemetry collection in kibana/stats in 7.11.

In fact, I think we could simplify this PR even more then. Is there any reason we couldn't simply stop collecting usage data starting Metricbeat 7.11?

[afharo]

And if internal users are tolerant to not having usage telemetry in case an end-user runs Metricbeat 7.11+ against a monitoring cluster that's < 7.11 (as @afharo explained in #22654 (comment)), then I'm ++ removing usage telemetry collection in kibana/stats in 7.11.

AFAIK, we should be OK in that scenario. The 7.11+ (in fact, 7.2+) production Kibana instance will be reporting their locally collected telemetry anyway (that's why we are after stopping Monitoring from doing so).

In fact, I think we could simplify this PR even more then. Is there any reason we couldn't simply stop collecting usage data starting Metricbeat 7.11?

I was a bit wary of simply deleting the piece of code 😅. But if we are all happy to do so, tomorrow I'll create a new PR with the removal and we can choose which one we like best :)

Thank you both @kobelb and @ycombinator for your thoughts on this!

[afharo]

@ycombinator I've created #22732 superseding this one. It fully removes the exclude_usage=false logic

[afharo]

Closing this PR in favour of #22732