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

[Monitoring] Add cluster metadata to cluster_stats docs #33860

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Sep 19, 2018

Resolves #33691.

This PR teaches Monitoring to collect cluster metadata, if any is set, and index it into cluster_stats docs in .monitoring-es-*.

After this PR, cluster_stats docs in .monitoring-es-* will contain an additional top-level cluster_settings field like so:

{
   ...
   "cluster_settings": {
     "cluster": {
       "metadata": {
         ...
       }
     }
   }
}

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@ycombinator ycombinator force-pushed the x-pack/monitoring/collect-cluster-metadata branch from 65263ff to 8f3dfb3 Compare September 19, 2018 15:20
@ycombinator
Copy link
Contributor Author

Chatting with @pickypg off-PR, he suggested I could simply grab the settings from cluster state, which is already available to the ClusterStatsCollector. I've started going down this much simpler path in 8d86d4f. Thanks @pickypg!

@ycombinator ycombinator added review and removed WIP labels Sep 19, 2018
@ycombinator
Copy link
Contributor Author

This is ready for an initial review now.

As discussed in #33691, I'm deliberately not adding anything to the index template or .monitoring-es-6-*, since we don't want to actually index the new cluster_settings field, just store it.

I don't believe we need any version checking in the code (owing to cluster metadata only being added in 6.5), but I'd like reviewers to confirm that.

I still need to add/update tests.

Copy link
Contributor

@gwbrown gwbrown left a comment

Choose a reason for hiding this comment

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

LGTM on the settings front (given adding tests as mentioned), but you may want another signoff from someone more familiar with monitoring.

Copy link
Member

@pickypg pickypg left a comment

Choose a reason for hiding this comment

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

LGTM. You won't need version checks because none of this leaves the node in a way that requires serialization between nodes.

@ycombinator ycombinator force-pushed the x-pack/monitoring/collect-cluster-metadata branch from 81404dc to f5d204e Compare September 24, 2018 16:36
@ycombinator ycombinator merged commit 9b86e9a into elastic:master Sep 25, 2018
@ycombinator ycombinator deleted the x-pack/monitoring/collect-cluster-metadata branch September 25, 2018 01:52
kcm pushed a commit that referenced this pull request Oct 30, 2018
* WIP

* Adding cluster metadata to cluster stats monitoring doc

* Fixing checkstyle errors

* Adding missing license header

* Updating tests

* Getting cluster settings from cluster state

* Removing more unnecessary changes

* Adding cluster metadata settings to cluster_stats docs

* Updating test to include cluster metadata

* Fixing checkstyle

* Guarding against NPE

* Updating test fixture
ycombinator added a commit to ycombinator/elasticsearch that referenced this pull request Nov 1, 2018
* WIP

* Adding cluster metadata to cluster stats monitoring doc

* Fixing checkstyle errors

* Adding missing license header

* Updating tests

* Getting cluster settings from cluster state

* Removing more unnecessary changes

* Adding cluster metadata settings to cluster_stats docs

* Updating test to include cluster metadata

* Fixing checkstyle

* Guarding against NPE

* Updating test fixture
ycombinator added a commit that referenced this pull request Nov 5, 2018
)

Backport of #33860 and #34040.

This PR teaches Monitoring to collect cluster metadata, if any is set, and index it into `cluster_stats` docs in `.monitoring-es-*`.

After this PR, `cluster_stats` docs in `.monitoring-es-*` will contain an additional top-level `cluster_settings` field like so:

```
{
   ...
   "cluster_settings": {
     "cluster": {
       "metadata": {
         ...
       }
     }
   }
}
```
ycombinator added a commit to elastic/beats that referenced this pull request Nov 8, 2018
…data to cluster_stats docs (#8990)

Cherry-pick of PR #8445 to 6.x branch. Original message: 

Porting over elastic/elasticsearch#33860 to the Metricbeat Elasticsearch module (X-Pack Monitoring code path).

This PR teaches Elasticsearch X-Pack Monitoring to collect cluster metadata, if any is set, and index it into `cluster_stats` docs in `.monitoring-es-6-mb-*`.

After this PR, `cluster_stats` docs in `.monitoring-es-6-mb-*` will contain an additional top-level `cluster_settings` field like so:

```
{
   ...
   "cluster_settings": {
     "cluster": {
       "metadata": {
         ...
       }
     }
   }
}
```
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.

5 participants