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

Restructure beats config options #4716

Merged
merged 9 commits into from
Jul 25, 2017

Conversation

dedemorton
Copy link
Contributor

@dedemorton dedemorton commented Jul 19, 2017

This resolves #4422 plus applies the same structural changes to all the Beats documentation. Here's a link to the docs for preview (a bit outdated because I've pushed changes to the files since I published the example). Please ignore the fact that the URL says logstash. I kept kicking Firebase, but it wouldn't let me create and deploy to a new app.

This PR mostly covers structural and style changes. I also added some missing content, moved some stuff around, and changed some text to improve the flow. The only new content (other than some filler text) is what I've added to filebeat-modules-options.asciidoc. Other stuff that appears new was just moved from another file. I needed to move things to restructure the TOC.

Summary of changes:

Changes for all beats:

  • Removed the Configuration Options (Reference) container from the TOC
  • Got rid of the reference folder in the source (it was lame)
  • Moved all topics under config up one level in the hierarchy
  • Reorganized config topics and merged redundant info
  • Made some content changes to improve the flow of ideas
  • Changed headings from title case to sentence case (yay) to align new consistency guidelines that will come out soonish
  • Changed all level 2 headings and below to use verbs instead of gerunds to align with our new guidelines (verbs are a lot easier to scan).
  • Added code formatting (backticks) to config option names. This will make it easier for translators to identify text that shouldn't be translated.

Additional changes for Filebeat:

  • Added a new topic "Specify which modules to run" (filebeat-modules-options.asciidoc) but I didn't hook it up because I'm on the fence about whether we should discuss the modules config under "Configuring Filebeat"

Additional changes for Packetbeat:

  • Chunked section about protocols into separate topics (makes it easier to scan the TOC and see what we offer...plus that section was getting very long). The section is under Configuring Packetbeat > Specify which transaction protocols to monitor
  • Added a few simplistic examples to the protocol topics in case a user accesses the individual pages before seeing the overview. These examples should be fleshed out with more config options at some point maybe.

I lost track of the changes for the other beats, so I can't summarize them here.

@dedemorton dedemorton added docs review in progress Pull request is currently in progress. labels Jul 19, 2017
@dedemorton dedemorton force-pushed the restructure_filebeat branch from 7f4e9e0 to 0b75dab Compare July 19, 2017 23:57
@dedemorton dedemorton force-pushed the restructure_filebeat branch from 0b75dab to d565b57 Compare July 21, 2017 22:41
@dedemorton dedemorton changed the title Restructure Filebeat and Libbeat config options Restructure beats config options Jul 22, 2017
@dedemorton dedemorton force-pushed the restructure_filebeat branch from e454ce6 to 822975f Compare July 22, 2017 05:08
@@ -0,0 +1,11 @@
[[packetbeat-reference-yml]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying an experiment here. Is it worthwhile to provide this in the docs? This is sort of placeholder for a real quick reference that lists all config options.

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, let's add the reference file to all Beats docs.

@dedemorton dedemorton removed the in progress Pull request is currently in progress. label Jul 25, 2017
@dedemorton
Copy link
Contributor Author

dedemorton commented Jul 25, 2017

@monicasarbu This is ready for a review. I have a few issues I need to address (either before or after the PR is merged):

  • Decide how to handle the include directive that pulls in the reference.yml file. Do you think it's a problem if we point directly to the file under beats/beatname? Do I need to worry about new config options being added in a patch release, or is that so unusual that I needn't worry? Making a copy of the reference file is not ideal IMO because the copy will need to be maintained.

  • If we point to the files under beats/beatname, I need to update the sources: entry in the conf.yml after this PR is merged so that doc build can find the referenced

  • I should update the doc collector script for each beat to make sure the capitalization matches our new standard.

@monicasarbu
Copy link
Contributor

@dedemorton I don't see any issue if you point directly to the reference file.

@@ -6728,7 +6728,7 @@ The current number of idle client connections waiting for a request.


[[exported-fields-php_fpm]]
== php_fpm Fields
== PHP-FPM 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.

@monicasarbu In my edits, I'm being consistent with our general approach of using the name of the product/service that's being monitored (for example, PHP-FPM) when we describe the module. Otherwise, especially in headings, it sort of looks like we are misspelling product names. But I'm wondering...do you think this might cause people to use the wrong module names in their configs (like php-fpm instead of php_fpm)? We do show a config example, so I might be overthinking this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say to leave the name of the module as it shows in the configuration (with _ instead of -) as otherwise might be confusing for the user.

@dedemorton dedemorton merged commit 52850a1 into elastic:master Jul 25, 2017
dedemorton added a commit to dedemorton/beats that referenced this pull request Jul 26, 2017
* Restructure Filebeat and Libbeat config options

* Restructure Heartbeat config options

* Restructure Packetbeat config options

* Experiment: show packetbeat.reference.yml in the docs

* Restructure Winlogbeat config options

* Restructure Metricbeat config options

* Restructure Auditbeat config options

* Add reference configs to the docs

* Move include statement for general options for consistency
dedemorton added a commit that referenced this pull request Jul 27, 2017
* Restructure Filebeat and Libbeat config options

* Restructure Heartbeat config options

* Restructure Packetbeat config options

* Experiment: show packetbeat.reference.yml in the docs

* Restructure Winlogbeat config options

* Restructure Metricbeat config options

* Restructure Auditbeat config options

* Add reference configs to the docs

* Move include statement for general options for consistency
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* Restructure Filebeat and Libbeat config options

* Restructure Heartbeat config options

* Restructure Packetbeat config options

* Experiment: show packetbeat.reference.yml in the docs

* Restructure Winlogbeat config options

* Restructure Metricbeat config options

* Restructure Auditbeat config options

* Add reference configs to the docs

* Move include statement for general options for consistency
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.

Simplify the section about Configuring Filebeat
2 participants