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

[Doc] added description of monitoring setting write_direct #11586

Closed

Conversation

andsel
Copy link
Contributor

@andsel andsel commented Feb 6, 2020

Describe the setting xpack.monitoring.collection.write_direct.enabled introduced with PR #11541. The behaviour of this flag is to enable the internal collectors to shape the metrics documents in a way to skip the Elasticsearch's monitoring exporter. Essentially we do the same job the exporter already does, and we switch the endpoint we use to push data from _monitoring/bulk to _bulk/monitoring

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

What you have here builds cleanly and LGTM.
Should this new setting be added here:

# ------------ X-Pack Settings (not applicable for OSS build)--------------
?

@andsel
Copy link
Contributor Author

andsel commented Feb 6, 2020

I've added, but in this other PR #11541

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

As discussed in the code issue, I think we should make sure we are consistent with the rest of the stack before committing the changes


Forces the internal collectors to write directly to the monitoring {es} without
traversing the monitoring exporter provided by {es}. Disabled by default, this setting
is intermediate step in deprecating the internal collectors.
Copy link
Member

Choose a reason for hiding this comment

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

Adding to my comment with the code changes, I suspect we are also going to need to do some more extensive documentation changes for this update to mirror the docs for filebeat monitoring, where there are three options for monitoring: Legacy collection (monitoring data sent via production cluster), Internal collection (monitoring data sent direct to monitoring cluster) and metricbeat collection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed a little bit the documenatiton part related to xpack monitoring, let me know what do you think @karenzone

@andsel andsel force-pushed the feature/document_write_direct_setting branch from 1f0d711 to bfd7a04 Compare February 17, 2020 13:48
@karenzone karenzone self-requested a review February 18, 2020 20:22
Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

I think the monitoring docs need a thorough rework. The changes are more extensive than just the changes to settings, and I need to update the docs to specify the cluster to send monitoring data, etc.

I'd like to work on this as a separate PR, and get input from you, Rob, others on the structure and content. WDYT?

@karenzone karenzone self-requested a review February 18, 2020 22:27
@robbavey
Copy link
Member

@karenzone If you are willing to do that, that would be awesome. I think this PR is a good jumping off point, but there are a few areas in the (new) internal collection section that could do with a bit of clarification/clean up - the docs still reference sending to the production cluster, and it might be nice to talk about using the new cloud.id settings to point to a cloud monitoring cluster as well as tweak a few other areas that might be confusing (how to set cluster_uuid, etc)

@andsel
Copy link
Contributor Author

andsel commented Feb 19, 2020

I agree with you

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

Yes! Much good in this PR. I'll be branching from here and using as a starting point for the doc reorg.

@andsel
Copy link
Contributor Author

andsel commented Apr 17, 2020

Close this because work has been moved into #11714 and #11789

@andsel andsel closed this Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants