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

Check ES for CCR feature availability #17073

Merged
merged 11 commits into from
Mar 18, 2020
Merged

Check ES for CCR feature availability #17073

merged 11 commits into from
Mar 18, 2020

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Mar 18, 2020 •
edited
Loading

What does this PR do?

Improves the check for CCR availability by not just checking the license level but also checking if the CCR feature is actually available in Elasticsearch.

Why is it important?

In some environments, like Elastic Cloud, the CCR feature is not available in Elasticsearch despite the license level being high enough. In such environments we currently throw errors in the log. This change will fix the error logging behavior.

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

How to test this PR locally

  1. Spin up a deployment on Elastic Cloud.

  2. Grab the Elasticsearch Endpoint URL.

  3. Grab the password for the elastic user.

  4. Enable the elasticsearch-xpack Metricbeat module.

    ./metricbeat modules enable elasticsearch-xpack

  5. Edit modules.d/elasticsearch-xpack.yml.

    • Set hosts to the Elasticsearch Endpoint URL from step 2.
    • Set username to elastic.
    • Set password to the password from step 3.

  6. For ease of testing, edit metricbeat.yml and set output.elasticsearch.enabled: false, output.file.path: /tmp, and output.file.filename: metricbeat.

  7. Start Metricbeat and watch the logs being output to the console.

    metricbeat -e

  8. Assert that the following message does not appear:

    Error fetching data for metricset elasticsearch.ccr: HTTP error 405 in : 405 Method Not Allowed

  9. Stop Metricbeat.

  10. Restart Metricbeat with debug logging for the Elasticsearch CCR metricset and watch the logs being output to the console.

    metricbeat -d 'elasticsearch.ccr' -e

  11. Assert that the following message does appear:

    DEBUG   [elasticsearch.ccr]     ccr/ccr.go:86   the CCR feature is not enabled on your Elasticsearch cluster.

Related issues

@andresrc andresrc added [zube]: Inbox Team:Integrations Label for the Integrations team labels Mar 18, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@ycombinator
Copy link
Contributor Author

Hey @andresrc I've removed the [zube]: In Review label as this is a draft PR (not yet ready for review yet). Is it possible to skip draft PRs when moving them to In Review in Zube?

@ycombinator ycombinator marked this pull request as ready for review March 18, 2020 14:36
@ycombinator ycombinator added [zube]: In Review v7.7.0 v8.0.0 needs_backport PR is waiting to be backported to other branches. labels Mar 18, 2020
@ycombinator ycombinator requested a review from a team March 18, 2020 14:39
@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

@ycombinator ycombinator added the test-plan Add this PR to be manual test plan label Mar 18, 2020
kaiyan-sheng
kaiyan-sheng approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

Overall, it looks good to me. One question, is there any documentation related place need to be added/changed?

@ycombinator
Copy link
Contributor Author

One question, is there any documentation related place need to be added/changed?

Yeah, probably a good idea to mention something on https://www.elastic.co/guide/en/beats/metricbeat/current/metricbeat-metricset-elasticsearch-ccr.html. Thanks, I will update this PR with the necessary doc changes.

dedemorton
dedemorton requested changes Mar 18, 2020
View reviewed changes
metricbeat/module/elasticsearch/ccr/_meta/docs.asciidoc
@@ -1,3 +1,8 @@
This is the `ccr` metricset of the Elasticsearch module. It interrogates the
Cross Cluster Replication Stats API endpoint to fetch information about shards
in the Elasticsearch cluster that are participating in cross-cluster replication.
Cross-Cluster Replication Stats API endpoint to fetch metrics about cross-cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a sentence fragment. Maybe say something like:

"This metricset uses the Cross-Cluster Replication Stats API endpoint to fetch...."

metricbeat/module/elasticsearch/ccr/_meta/docs.asciidoc Outdated
Cross Cluster Replication Stats API endpoint to fetch information about shards
in the Elasticsearch cluster that are participating in cross-cluster replication.
Cross-Cluster Replication Stats API endpoint to fetch metrics about cross-cluster
replication from the Elasticsearch clusters that are participating in cross-cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, but I'm trying to clean up the docs by using the correct attribute to resolve product names. So you should use {es} instead of Elasticsearch throughout this topic.

metricbeat/module/elasticsearch/ccr/_meta/docs.asciidoc Outdated
replication from the Elasticsearch clusters that are participating in cross-cluster
replication.

If the Elasticserach cluster does not have the cross-cluster replication feature
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest pruning a few words here and saying: "...does not have cross-cluster replication enabled..."

metricbeat/module/elasticsearch/ccr/_meta/docs.asciidoc Outdated
replication.

If the Elasticserach cluster does not have the cross-cluster replication feature
enabled, this metricset will not collect any metrics. A DEBUG log message about this
Copy link
Contributor

Choose a reason for hiding this comment

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

again, some pruning: "...will not collect metrics."

@ycombinator
Copy link
Contributor Author

@dedemorton I've made the suggested changes. Please re-review when you get a chance. Thanks!

@ycombinator
Copy link
Contributor Author

Travis CI is green and Jenkins CI failures are unrelated. Merging.

@ycombinator ycombinator merged commit fe01a5a into elastic:master Mar 18, 2020
@ycombinator ycombinator deleted the mb-es-ccr-cloud branch March 18, 2020 23:37
ycombinator added a commit to ycombinator/beats that referenced this pull request Mar 18, 2020
@ycombinator
Check ES for CCR feature availability (elastic#17073)
7a7a0d3 
* Add TODO

* Check if CCR feature is available in ES

* Allow tests to disable license caching

* Add unit tests

* Separate message

* Adding license header

* Adding CHANGELOG entry

* Check enabled, not available

* Renaming var

* Adding documentation

* Doc feedback fixes
@ycombinator ycombinator mentioned this pull request Mar 18, 2020
Merged
@ycombinator ycombinator removed the needs_backport PR is waiting to be backported to other branches. label Mar 18, 2020
ycombinator added a commit that referenced this pull request Mar 19, 2020
@ycombinator
Check ES for CCR feature availability (#17073) (#17099)
6373f79 
* Add TODO

* Check if CCR feature is available in ES

* Allow tests to disable license caching

* Add unit tests

* Separate message

* Adding license header

* Adding CHANGELOG entry

* Check enabled, not available

* Renaming var

* Adding documentation

* Doc feedback fixes
@andresrc andresrc added test-plan-added This PR has been added to the test plan and removed [zube]: Done labels Mar 21, 2020
@ycombinator ycombinator mentioned this pull request Apr 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaiyan-sheng kaiyan-sheng kaiyan-sheng approved these changes

@dedemorton dedemorton dedemorton approved these changes

Assignees
No one assigned
Labels
bug Feature:Stack Monitoring Metricbeat Metricbeat module Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Elasticsearch CCR metricset returns 405 errors when run against ES in Cloud
5 participants
@ycombinator @elasticmachine @kaiyan-sheng @dedemorton @andresrc