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

Support shipping directly to monitoring cluster #9260

Merged
merged 59 commits into from
Apr 3, 2019

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Nov 28, 2018

Currently all beats ship their x-pack monitoring data to the production Elasticsearch cluster by POSTing it to the custom _xpack/monitoring/_bulk Elasticsearch API endpoint.

In the future we want to support beats shipping their x-pack monitoring data directly to a monitoring Elasticsearch cluster (which could be the same as their production cluster but doesn't necessarily have to be) by POSTing it to the regular _bulk Elasticsearch API endpoint.

This PR introduces this "direct shipping" feature. Concretely:

  • It introduces a new class of settings named monitoring.* that will parallel the current xpack.monitoring.* settings. If the user has set xpack.monitoring.* the beat will continue to ship to the production cluster as it does today (but also emit a deprecation warning about these settings). However, if the user has set the monitoring.*, the beat will instead ship directly to the monitoring cluster.

  • It modifies the Elastisearch reporter to decide where to ship the data (production cluster, _xpack/monitoring/_bulk API endpoint -or- monitoring cluster, _bulk API endpoint).

  • If data is being shipped directly to the monitoring cluster AND the beat is configured to use an elasticsearch output, the Elasticsearch reporter will inject this cluster_uuid of the elasticsearch output cluster into the monitoring data event before publishing it.

@ycombinator ycombinator added in progress Pull request is currently in progress. libbeat Feature:Stack Monitoring labels Nov 28, 2018
@ycombinator
Copy link
Contributor Author

ycombinator commented Nov 28, 2018

@urso @ruflin This PR is very much a WIP but I'd like some early feedback from you to know if I'm on the right track (since this is my first non-trivial PR to libbeat code).

Specifically at this point I'm wondering what you think of how/when I'm getting the cluster_uuid from the ES output cluster and then how I can properly use it in the ES reporter to add it to the monitoring event.

Note that I haven't yet added the logic to ship to the regular _bulk API on the monitoring cluster instead of the custom _xpack/monitoring/_bulk API on the production cluster. I've started implementing this but it's not quite working yet.

libbeat/cmd/instance/beat.go Outdated Show resolved Hide resolved
libbeat/cmd/instance/beat.go Outdated Show resolved Hide resolved
libbeat/cmd/instance/beat.go Show resolved Hide resolved
libbeat/cmd/instance/beat.go Outdated Show resolved Hide resolved
libbeat/cmd/instance/beat.go Outdated Show resolved Hide resolved
libbeat/monitoring/report/elasticsearch/client.go Outdated Show resolved Hide resolved
libbeat/monitoring/report/report.go Outdated Show resolved Hide resolved
@ycombinator ycombinator requested a review from a team as a code owner January 18, 2019 06:15
@ycombinator ycombinator force-pushed the direct-monitoring branch 2 times, most recently from 170e5fe to 76013d0 Compare January 21, 2019 12:40
@ycombinator ycombinator added review v7.0.0 v6.7.0 and removed in progress Pull request is currently in progress. labels Jan 21, 2019
@ycombinator
Copy link
Contributor Author

ycombinator commented Jan 21, 2019

@urso @ruflin I've just finished the pending work on this PR and it is ready for your review again. Thanks!

@ycombinator ycombinator changed the title [WIP] Support shipping directly to monitoring cluster Support shipping directly to monitoring cluster Jan 21, 2019
@ruflin
Copy link
Member

ruflin commented Jan 22, 2019

Is this already ready to go? Asking because I see elastic/kibana#27595 is still open?

If someone configures monitoring.* with the same values as in x-pack.monitoring the outcome will be identical?

What is the expected behaviour if someone connects to an OSS cluster? Will the data be shipped or not?

@ycombinator
Copy link
Contributor Author

ycombinator commented Jan 22, 2019

Is this already ready to go? Asking because I see elastic/kibana#27595 is still open?

This is ready for review. But, as the note at the top of this PR says, it should not be merged until elastic/kibana#27595 is merged first. Basically I would like to have this PR reviewed and ready to merge so we can just push the button as soon as elastic/kibana#27595 gets merged.

If someone configures monitoring.* with the same values as in x-pack.monitoring the outcome will be identical?

Yes, the end result will be the same: exactly the same documents will end up in .monitoring-beats-* indices. The difference will be in how they get there. If the user configures xpack.monitoring.*, they will get there via the POST _xpack/monitoring/bulk API on the production ES cluster. However, if the user configures monitoring.*, they will get there via the POST _bulk API on the monitoring cluster.

[EDIT] This means that we expect — and want — users to use the monitoring.* settings and pointing to the Monitoring cluster, instead of using the xpack.monitoring.* settings and pointing to the Production cluster. This "want" is reflected in the deprecation log messages implemented by this PR.

What is the expected behaviour if someone connects to an OSS cluster? Will the data be shipped or not?

The data should not be shipped. I would expect the checks in this method to fail and corresponding errors to be logged:

func (c *publishClient) Connect() error {
debugf("Monitoring client: connect.")
params := map[string]string{
"filter_path": "features.monitoring.enabled",
}
status, body, err := c.es.Request("GET", "/_xpack", "", params, nil)
if err != nil {
return fmt.Errorf("X-Pack capabilities query failed with: %v", err)
}
if status != 200 {
return fmt.Errorf("X-Pack capabilities query failed with status code: %v", status)
}
resp := struct {
Features struct {
Monitoring struct {
Enabled bool
}
}
}{}
if err := json.Unmarshal(body, &resp); err != nil {
return err
}
if !resp.Features.Monitoring.Enabled {
debugf("XPack monitoring is disabled.")
return errNoMonitoring
}
debugf("XPack monitoring is enabled")
return nil
}

[EDIT] I just confirmed this by actually running this PR with an OSS ES distribution. The same INFO messages that are emitted by master in this scenario when the user uses xpack.monitoring.* are emitted by this PR when the user uses either xpack.monitoring.* or monitoring.*:

2019-01-22T05:49:11.839-0800    INFO    [monitoring]    elasticsearch/elasticsearch.go:234      Failed to connect to Elastic X-Pack Monitoring. Either Elasticsearch X-Pack monitoring is not enabled or Elasticsearch is not available. Will keep retrying.

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Left a few minor comments. I think this definitively deserves a changelog entry.

For the testing: It would be great to have system tests for this that ingest some data with both options and then check if the data is there.

libbeat/cmd/instance/beat.go Outdated Show resolved Hide resolved
libbeat/monitoring/report/elasticsearch/client.go Outdated Show resolved Hide resolved
libbeat/monitoring/report/report.go Outdated Show resolved Hide resolved
libbeat/monitoring/report/report.go Outdated Show resolved Hide resolved
libbeat/monitoring/report/report.go Outdated Show resolved Hide resolved
@ycombinator ycombinator merged commit 01ff3e4 into elastic:master Apr 3, 2019
@cachedout
Copy link
Contributor

🎉 🍾

@ycombinator ycombinator deleted the direct-monitoring branch April 3, 2019 19:13
ycombinator added a commit that referenced this pull request Apr 9, 2019
)

Beats now have the ability to send their monitoring data directly to the **monitoring** Elasticsearch cluster (#9260). This PR updates the default and reference configuration files for all Beats with the new monitoring settings. It also updates the configuration template used by Central Management.
ycombinator added a commit to elastic/elasticsearch that referenced this pull request Apr 16, 2019
Traditionally we have [recommended](https://www.elastic.co/guide/en/beats/filebeat/current/monitoring.html) that Beats send their monitoring data to the **production** Elasticsearch cluster. Beats do this by calling the `POST _monitoring/bulk` API. When Security is enabled this API call requires the `cluster:admin/xpack/monitoring/bulk` privilege. The built-in `beats_system` role has this privilege.

[Going forward](elastic/beats#9260), Beats will be able to send their monitoring data directly to the **monitoring** Elasticsearch cluster. Beats will do this by calling the regular `POST _bulk` API. When Security is enabled this API call requires the `indices:data/write/bulk` privilege. Further, the call has to be able to create any indices that don't exist.

This PR expands the built-in `beats_system` role's privileges. Specifically, it adds index-level `write` and `create_index` privileges for `.monitoring-beats-*` indices. 

This will allow Beats users to continue using the `beats_system` role for the new direct monitoring route when Security is enabled.
ycombinator added a commit to ycombinator/elasticsearch that referenced this pull request Apr 16, 2019
Traditionally we have [recommended](https://www.elastic.co/guide/en/beats/filebeat/current/monitoring.html) that Beats send their monitoring data to the **production** Elasticsearch cluster. Beats do this by calling the `POST _monitoring/bulk` API. When Security is enabled this API call requires the `cluster:admin/xpack/monitoring/bulk` privilege. The built-in `beats_system` role has this privilege.

[Going forward](elastic/beats#9260), Beats will be able to send their monitoring data directly to the **monitoring** Elasticsearch cluster. Beats will do this by calling the regular `POST _bulk` API. When Security is enabled this API call requires the `indices:data/write/bulk` privilege. Further, the call has to be able to create any indices that don't exist.

This PR expands the built-in `beats_system` role's privileges. Specifically, it adds index-level `write` and `create_index` privileges for `.monitoring-beats-*` indices. 

This will allow Beats users to continue using the `beats_system` role for the new direct monitoring route when Security is enabled.
ycombinator added a commit to elastic/elasticsearch that referenced this pull request Apr 16, 2019
Traditionally we have [recommended](https://www.elastic.co/guide/en/beats/filebeat/current/monitoring.html) that Beats send their monitoring data to the **production** Elasticsearch cluster. Beats do this by calling the `POST _monitoring/bulk` API. When Security is enabled this API call requires the `cluster:admin/xpack/monitoring/bulk` privilege. The built-in `beats_system` role has this privilege.

[Going forward](elastic/beats#9260), Beats will be able to send their monitoring data directly to the **monitoring** Elasticsearch cluster. Beats will do this by calling the regular `POST _bulk` API. When Security is enabled this API call requires the `indices:data/write/bulk` privilege. Further, the call has to be able to create any indices that don't exist.

This PR expands the built-in `beats_system` role's privileges. Specifically, it adds index-level `write` and `create_index` privileges for `.monitoring-beats-*` indices. 

This will allow Beats users to continue using the `beats_system` role for the new direct monitoring route when Security is enabled.
@ycombinator ycombinator mentioned this pull request Apr 27, 2019
3 tasks
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Traditionally we have [recommended](https://www.elastic.co/guide/en/beats/filebeat/current/monitoring.html) that Beats send their monitoring data to the **production** Elasticsearch cluster. Beats do this by calling the `POST _monitoring/bulk` API. When Security is enabled this API call requires the `cluster:admin/xpack/monitoring/bulk` privilege. The built-in `beats_system` role has this privilege.

[Going forward](elastic/beats#9260), Beats will be able to send their monitoring data directly to the **monitoring** Elasticsearch cluster. Beats will do this by calling the regular `POST _bulk` API. When Security is enabled this API call requires the `indices:data/write/bulk` privilege. Further, the call has to be able to create any indices that don't exist.

This PR expands the built-in `beats_system` role's privileges. Specifically, it adds index-level `write` and `create_index` privileges for `.monitoring-beats-*` indices. 

This will allow Beats users to continue using the `beats_system` role for the new direct monitoring route when Security is enabled.
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
…stic#11677)

Beats now have the ability to send their monitoring data directly to the **monitoring** Elasticsearch cluster (elastic#9260). This PR updates the default and reference configuration files for all Beats with the new monitoring settings. It also updates the configuration template used by Central Management.
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