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

[Metricbeat/Kibana/stats] Enforce exclude_usage=true #22732

Merged

Conversation

afharo
Copy link
Member

@afharo afharo commented Nov 24, 2020

What does this PR do?

Changes to the Metricbeat's kibana module: it enforces exclude_usage=true (only for the versions it's accepted).

The reason for this change is to disable the usage collection via the /api/stats endpoint, as it is no longer needed for their initial Telemetry purposes.

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

  • 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=true&legacy=true&exclude_usage=false request (the last query parameter is always true).

Related issues

Use cases

Explained in the issue #22651

Screenshots

Logs

@afharo afharo added enhancement Metricbeat Metricbeat Feature:Stack Monitoring v8.0.0 Team:Services (Deprecated) Label for the former Integrations-Services team v7.11.0 labels Nov 24, 2020
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Nov 24, 2020
@afharo afharo changed the title [Metricbeat/Kibana/stats] exclude_usage=true by default [Metricbeat/Kibana/stats] Enforce exclude_usage=true when accepted by the version Nov 24, 2020
@afharo afharo changed the title [Metricbeat/Kibana/stats] Enforce exclude_usage=true when accepted by the version [Metricbeat/Kibana/stats] Enforce exclude_usage=true Nov 24, 2020
@afharo
Copy link
Member Author

afharo commented Nov 24, 2020

jenkins, test this please

@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 24, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #22732 updated]

  • Start Time: 2020-11-25T10:40:51.095+0000

  • Duration: 50 min 3 sec

Test stats 🧪

Test Results
Failed 0
Passed 2262
Skipped 506
Total 2768

Steps errors 2

Expand to view the steps failures

Terraform Apply on x-pack/metricbeat/module/aws

  • Took 0 min 14 sec . View more details on here

Terraform Apply on x-pack/metricbeat/module/aws

  • Took 0 min 15 sec . View more details on here

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 2262
Skipped 506
Total 2768

@afharo afharo marked this pull request as ready for review November 24, 2020 17:46
@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

@elasticmachine
Copy link
Collaborator

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

return err
}
m.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.

With this change we no longer need the comment above if m.isUsageExcludable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well spotted! I updated it to explain what's actually happening now :)
Thank you!

usageLastCollectedOn: test.usageLastCollectedOn,
usageNextCollectOn: test.usageNextCollectOn,
}
func TestFetchNoExcludeUsage(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding this test. 👍

Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

Just left a small comment about an outdated comment. Otherwise LGTM. Nice work!

@ycombinator ycombinator added the needs_backport PR is waiting to be backported to other branches. label Nov 25, 2020
@afharo afharo merged commit 144e94b into elastic:master Nov 25, 2020
@afharo afharo deleted the metricbeat/kibana/stats/exclude_usage_removal branch November 25, 2020 12:15
afharo added a commit to afharo/beats that referenced this pull request Nov 25, 2020
v1v added a commit to v1v/beats that referenced this pull request Dec 2, 2020
…-issues

* upstream/master: (41 commits)
  Fix version parser regex for packaging (elastic#22581)
  Fix local_dynamic documentation and add providers inline doc. (elastic#22657)
  fix: use proper param name for e2e tests (elastic#22836)
  [Heartbeat] Fix exit on disabled monitor (elastic#22829)
  Update Golang to 1.14.12 (elastic#22790)
  docs: fix setup.template.overwrite typos (elastic#22804)
  Add docs section for ECS EC2 monitoring (elastic#22784)
  Fixing logic to keep list of unique cluster UUIDs (elastic#22808)
  Skip somewhat flaky UDP system test on Windows (elastic#22810)
  Fix polling node when it is not ready and monitor by hostname (elastic#22666)
  Skip Filebeat test_shutdown on windows 7 (elastic#22797)
  Make monitoring Namespace thread-safe (elastic#22640)
  Drop pkt_dstaddr and pkt_srcaddr when equals to "-" (elastic#22721)
  Add support for reading from UNIX datagram sockets (elastic#22699)
  Fix export dashboard command from Elastic Cloud (elastic#22746)
  Skip flaky winlogbeat test on Windows-7 (elastic#22754)
  Missing `>` (elastic#22763) (elastic#22766)
  Fix k8s watcher issue when node access to list nodes and ns (elastic#22714)
  [Metricbeat/Kibana/stats] Enforce `exclude_usage=true` (elastic#22732)
  Avoid sending non-numeric floats in cloud foundry integrations (elastic#22634)
  ...
v1v added a commit to v1v/beats that referenced this pull request Dec 2, 2020
…dows-7

* upstream/master: (41 commits)
  Fix version parser regex for packaging (elastic#22581)
  Fix local_dynamic documentation and add providers inline doc. (elastic#22657)
  fix: use proper param name for e2e tests (elastic#22836)
  [Heartbeat] Fix exit on disabled monitor (elastic#22829)
  Update Golang to 1.14.12 (elastic#22790)
  docs: fix setup.template.overwrite typos (elastic#22804)
  Add docs section for ECS EC2 monitoring (elastic#22784)
  Fixing logic to keep list of unique cluster UUIDs (elastic#22808)
  Skip somewhat flaky UDP system test on Windows (elastic#22810)
  Fix polling node when it is not ready and monitor by hostname (elastic#22666)
  Skip Filebeat test_shutdown on windows 7 (elastic#22797)
  Make monitoring Namespace thread-safe (elastic#22640)
  Drop pkt_dstaddr and pkt_srcaddr when equals to "-" (elastic#22721)
  Add support for reading from UNIX datagram sockets (elastic#22699)
  Fix export dashboard command from Elastic Cloud (elastic#22746)
  Skip flaky winlogbeat test on Windows-7 (elastic#22754)
  Missing `>` (elastic#22763) (elastic#22766)
  Fix k8s watcher issue when node access to list nodes and ns (elastic#22714)
  [Metricbeat/Kibana/stats] Enforce `exclude_usage=true` (elastic#22732)
  Avoid sending non-numeric floats in cloud foundry integrations (elastic#22634)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature:Stack Monitoring Metricbeat Metricbeat needs_backport PR is waiting to be backported to other branches. Team:Services (Deprecated) Label for the former Integrations-Services team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Metricbeat][Kibana][stats] Stop collecting usage
3 participants