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] Remove xpack enabled flag on ES, Logstash, Beats and Kibana #24427

Merged
merged 70 commits into from
Apr 27, 2021

Conversation

sayden
Copy link
Contributor

@sayden sayden commented Mar 8, 2021

What does this PR do?

Elasticsearch, Kibana, Logstash and Beat modules has a flag called xpack.enabled: true/false that can be activated to ensure that all metrics required on Stack Monitoring UI in Kibana are included in events from those modules. Before that, only a subset of the metrics were included.

At the same time, all metrics have been "refactored" to be fully compatible with ECS and the common structure of Metricbeat events.

Finally, metrics will appear in indices of Metricbeat (the good ol' metricbeat-* indices) instead of .monitoring indices. This way users will also be able to build their own dashboards around those modules more easily.

xpack.enabled: true flag will exist for some time more to help users activate the Metricbeat modules required by SM UI in Kibana.

Why is it important?

ECS is a benefit for all users and Elasticsearch, Kibana, Logstash and Beat modules weren´t publishing events following the schema yet. OSS users will also benefit from this change, as the metrics will now be available in metricbeat-* indices.

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.

How to test this PR locally

To test this PR locally, a SNAPSHOT version of Metricbeat and Kibana is necessary. Launch Elasticsearch, Kibana and Logstash and setup the same Metricbeat modules to monitor them. Metrics from those modules should appear in metricbeat-* indices and the Stack Monitoring UI in Kibana should work without issues.

To test Beats, you need to setup another Metricbeat or use this same one to monitor itself.

Related issues

ycombinator and others added 30 commits August 13, 2020 20:58
…hout xpack.enabled flag (#19747)

* Make node_stats ignore xpack.enabled: true for the POC

* Index more fields

* WIP: add fields to fields.yml

* Fleshing out alias fields

* Adding more fields to mapping

* Marking optional fields

* Fixing datatypes

* Fixing formatting

* Fixing alias field definitions

* More field fixes

* Making cgroups metrics collection optional

* Marking os.load_avg as optional for Windows

* Aliasing type => metricset.name

* Aliasing source_node fields

* Aliasing timestamp => @timestamp

* Removing field alias mappings for unused fields

* Removing unnecessary fields from mapping

* Updating generated files

* Reducing visibility of isMaster function

* Adding methods to metricset

* Refactoring for testability

* Fixing formatting

* Making indices.bulk and thread_pool.write fields optional for BC

* Deleting sample file from unsupported ES version

* Remove xpack.enabled code path!!!

* Updating xpack unit test

* Updating python system tests for xpack

* Remove top-level type field

* Removing data-xpack.json

* Updating data.json

* Not collecting unmapped fields

* Fixing formatting
…0613)

* Auto-include node_stats metricset when xpack.enabled: true is set

* Fixing some tests

* Try to fix python system test

* Fixing len check

* Fixing monitoring index type for node_stats metricset

* Account for node_stats docs being indexed into metricbeat-*

* Debugging

* More debugging

* Debugging

* Updating integration test

* Fixing test code
…eature/mb/elasticsearch/beats_state_xpack_flag
@sayden
Copy link
Contributor Author

sayden commented Apr 3, 2021

/test

1 similar comment
@sayden
Copy link
Contributor Author

sayden commented Apr 3, 2021

/test

@sayden
Copy link
Contributor Author

sayden commented Apr 8, 2021

/test

@chrisronline
Copy link
Contributor

chrisronline commented Apr 13, 2021

Here are the changes I needed to make to get beats/apm working: https://gist.github.com/chrisronline/f18740f00601251f979cec3dccbac320

At a high level, the apm-server aliases were all pointing to apm_server when it's really apm-server. I just kept it apm-server in both places but feel free to change it and lemme know what you decide.

I also noticed the version was not properly passed for ES cluster_stats so that change is in the diff too

…stack-monitoring-mb-ecs

# Conflicts:
#	metricbeat/module/logstash/logstash_integration_test.go
@mergify
Copy link
Contributor

mergify bot commented Apr 20, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feature-stack-monitoring-mb-ecs upstream/feature-stack-monitoring-mb-ecs
git merge upstream/master
git push upstream feature-stack-monitoring-mb-ecs

…stack-monitoring-mb-ecs

# Conflicts:
#	metricbeat/module/elasticsearch/index/data_xpack.go
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.

I skimmed through the changes in this PR as it's quite large. In any case, most of these changes were reviewed in smaller PRs that targeted the feature-stack-monitoring-mb-ecs branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants