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

Add the cluster_uuid field to Enterprise search stats metricset #28287

Merged
merged 7 commits into from
Oct 18, 2021

Conversation

kovyrin
Copy link
Contributor

@kovyrin kovyrin commented Oct 6, 2021

What does this PR do?

This PR adds the cluster_uuid field to Enterprise search stats metricset to make it easier to correlate with health metricset events. Additionally, in both metricsets, we have moved the cluster_uuid field to the module level to make it easier to aggregate data across two event types in Stack monitoring and Kibana dashboards.

Why is it important?

This is a follow-up for the recent PR that added the new module and we have just realized that having a uuid in all of our fields would make correlating them in Kibana a lot easier and make those events easier to work with in Kibana monitoring plugin (that relies on cluster UUID to group stack components together).

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

  • Tested it against the recent Enterprise Search snapshot that has the cluster_uuid field in Stats API responses.

How to test this PR locally

  • Deploy Enterprise Search
  • Run metricbeat with the enterprisesearch module enabled
  • Check stats events produced by the enterprisesearch module and you'll see the new field.

Alternatively, one can use integration tests to make sure the changes work (I've added data tests changes needed).

Related issues

…ke it easier to correlate with health metricset events
@kovyrin kovyrin added enhancement Team:Integrations Label for the Integrations team backport-v7.16.0 Automated backport with mergify labels Oct 6, 2021
@kovyrin kovyrin self-assigned this Oct 6, 2021
@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 Oct 6, 2021
@kovyrin kovyrin requested review from sayden and carlosdelest October 6, 2021 16:03
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 6, 2021

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2021-10-13T13:54:06.060+0000

  • Duration: 107 min 31 sec

  • Commit: c2a8857

Test stats 🧪

Test Results
Failed 0
Passed 9504
Skipped 2539
Total 12043

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

@kovyrin
Copy link
Contributor Author

kovyrin commented Oct 6, 2021

Integration tests will keep failing until we have a 7.16-SNAPSHOT build of enterprise search with the changes I've made today.

@kovyrin
Copy link
Contributor Author

kovyrin commented Oct 12, 2021

/test

@kovyrin kovyrin marked this pull request as ready for review October 13, 2021 00:56
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@@ -31086,7 +31086,7 @@ type: keyword
--

[[exported-fields-enterprisesearch]]
== enterprisesearch fields
== Enterprise Search fields
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just noticed we can control this via our module metadata and decided to make it prettier

[float]
=== product_usage

Aggregate product usage statistics for the Enterprise Search deployment.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a few more fields from our stats endpoint to improve the Stack Monitoring experience

@@ -3,10 +3,6 @@
release: beta
description: Enterprise Search health
fields:
- name: cluster_uuid
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this to the module level

@kovyrin kovyrin requested a review from ruflin October 13, 2021 14:23
@kovyrin
Copy link
Contributor Author

kovyrin commented Oct 13, 2021

Thoroughly tested this while developing the Stack Monitoring integration and it works really well.

@richkuz
Copy link

richkuz commented Oct 13, 2021

👍 New naming changes and metrics look good to me. Nice work @kovyrin !

@sayden would you be willing to review this PR for us, too?

Copy link
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

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

Good! Moving UUID to top level makes sense, and the additional fields are great for monitoring 👍

@kovyrin
Copy link
Contributor Author

kovyrin commented Oct 14, 2021

This PR is blocking the testing Kibana team needs to do on the Stack Monitoring integration PR: elastic/kibana#114303 (comment). If we could get this reviewed sooner rather than later, it'd be hugely appreciated 🙇🏻

@richkuz
Copy link

richkuz commented Oct 15, 2021

With two of us having reviewed it in App Search, I feel comfortable with the change. GitHub says we can technically merge now, but we are guests in your house, Beats team :) . @sayden or @ruflin , any objection to us merging this now? We'd like to get a snapshot cooking before Monday if possible.

@ruflin
Copy link
Contributor

ruflin commented Oct 18, 2021

@masci Can you chime in on who should give a 👍 from your team?

@masci
Copy link

masci commented Oct 18, 2021

@richkuz I'm ok merging this even without reviews from my team, @carlosdelest already approved and you folks know better than us.

Feel free to fully own this module, should you have any issue/doubt we'll be happy to help but other than that, no need to include us in the development process.

Thanks for adding more features to the solution!

@kovyrin kovyrin merged commit 78bcab3 into elastic:master Oct 18, 2021
@kovyrin kovyrin deleted the add-ent-search-stats-cluster-uuid branch October 18, 2021 15:46
mergify bot pushed a commit that referenced this pull request Oct 18, 2021
* Add the cluster_uuid field to Enterprise search stats metricset to make it easier to correlate with health metricset events

* Make Enterprise Search module field docs prettier

* Updated the docs

* Move cluster_uuid to the module level to make it easier to filter those events in Stack monitoring

* Add product usage metrics to the stats event

* Update mb docs following the addition of new fields

* Remove duplicate uuid fields now that we put the uuid at the module level

(cherry picked from commit 78bcab3)

# Conflicts:
#	metricbeat/docs/fields.asciidoc
kovyrin added a commit that referenced this pull request Oct 18, 2021
…h stats metricset (#28505)

* Add the cluster_uuid field to Enterprise search stats metricset (#28287)

* Add the cluster_uuid field to Enterprise search stats metricset to make it easier to correlate with health metricset events

* Make Enterprise Search module field docs prettier

* Updated the docs

* Move cluster_uuid to the module level to make it easier to filter those events in Stack monitoring

* Add product usage metrics to the stats event

* Update mb docs following the addition of new fields

* Remove duplicate uuid fields now that we put the uuid at the module level

(cherry picked from commit 78bcab3)

# Conflicts:
#	metricbeat/docs/fields.asciidoc

* Fix the conflict

Co-authored-by: Oleksiy Kovyrin <oleksiy@kovyrin.net>
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
…tic#28287)

* Add the cluster_uuid field to Enterprise search stats metricset to make it easier to correlate with health metricset events

* Make Enterprise Search module field docs prettier

* Updated the docs

* Move cluster_uuid to the module level to make it easier to filter those events in Stack monitoring

* Add product usage metrics to the stats event

* Update mb docs following the addition of new fields

* Remove duplicate uuid fields now that we put the uuid at the module level
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify enhancement Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants