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 modules.d layout to getting started and fix Windows command examples #6941

Merged
merged 5 commits into from
May 8, 2018

Conversation

dedemorton
Copy link
Contributor

@dedemorton dedemorton commented Apr 25, 2018

Fixes #5512 and #6196

I've also changed the metricbeat getting started to use asciidoc attributes to resolve "metricbeat" and "Metricbeat". My goal is to convert all the topics over to using attributes, but I don't want to open a big PR for those changes because it's easier for me to check for mistakes if I make the changes in smaller batches.

Question for reviewers:

One thing that's missing here is advice on enabling/disabling metricsets. Do we want to mention that step in the getting started, or leave users to read the configuration reference docs if they want to change the defaults?

+
["source","sh",subs="attributes"]
----
docker run docker.elastic.co/beats/{beatname_lc}:{version} modules enable apache mysql
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this won't work, as docker doesn't store the container status between runs

Copy link
Contributor Author

@dedemorton dedemorton Apr 25, 2018

Choose a reason for hiding this comment

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

@exekias Yes, you're right. I thought it worked when I ran the command because it said:

Rhodas-MacBook-Pro:myscripts$ docker run docker.elastic.co/beats/metricbeat:6.2.4 modules enable apache mysql
Enabled apache
Enabled mysql

But now when I run docker run docker.elastic.co/beats/metricbeat:6.2.4 modules list, I see that only the system module is enabled.

So how do we recommend to docker users that the enable/disable modules?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably it's tricky, they would need to mount a modules.d folder or metricbeat.yml from the host,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@exekias Yah, that makes sense. I was trying to add platform-specific commands for all the platforms we currently show in the getting started docs, but I didn't think it through very carefully. Since Docker is a special beast, sounds like I just need to point to:

https://www.elastic.co/guide/en/beats/metricbeat/current/running-on-docker.html

Copy link
Contributor Author

@dedemorton dedemorton Apr 27, 2018

Choose a reason for hiding this comment

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

@exekias One question, tho...should we mention in the command reference that the modules command won't work with Docker images?

Edited: Kind of begs the question as to whether we should include docker instructions in the getting started.

Copy link
Member

Choose a reason for hiding this comment

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

I would skip the docker command and only link to the docs as the command alone spins up the container but will not provide the user any useful output.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok if we skip docker here

Copy link
Contributor Author

@dedemorton dedemorton May 7, 2018

Choose a reason for hiding this comment

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

@ruflin I pushed another commit that removes the run example (and points to the full topic about running on docker) because it sounds like it's not useful to run that command. If it looks good, feel free to squash and merge.

@dedemorton
Copy link
Contributor Author

@exekias I've replaced the docker example with a link to the extended topic. I think we need to do more work to improve the docs around running on containers, but that's a separate item on my to-do list. Should someone else (maybe @ruflin) review this topic? Would like to get it merged soon.

and the Apache HTTPD module:

[source, shell]
-------------------------------------
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why your removed this part? I remember I added it because in the early days of Metricbeat on discuss several times the question popped up on how to configure 2 modules or if it's possible to configure the same module twice. I was hoping this example here helps.

Copy link
Contributor Author

@dedemorton dedemorton May 3, 2018

Choose a reason for hiding this comment

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

@ruflin I removed it because I wanted the getting started guide to focus using the configs in the modules.d directory (using the enable modules command is a better getting started experience). I thought users would look at this topic for config metricbeat.yml config examples: https://www.elastic.co/guide/en/beats/metricbeat/master/configuration-metricbeat.html

Do you want me to add the deleted example to the ones shown in configuration-metricbeat.html, or do you think the examples that are there are good enough?

Copy link
Member

Choose a reason for hiding this comment

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

@dedemorton You are right, it's all there. Forgot that we also have there some good examples. +1 on removing it here.

@ruflin
Copy link
Member

ruflin commented May 3, 2018

Changes LGTM. Left one questions.

@ruflin ruflin merged commit a57a0cb into elastic:master May 8, 2018
@ruflin ruflin mentioned this pull request May 8, 2018
stevea78 pushed a commit to stevea78/beats that referenced this pull request May 20, 2018
…les (elastic#6941)

Fixes elastic#5512 and elastic#6196

I've also changed the metricbeat getting started to use asciidoc attributes to resolve "metricbeat" and "Metricbeat". My goal is to convert all the topics over to using attributes, but I don't want to open a big PR for those changes because it's easier for me to check for mistakes if I make the changes in smaller batches.
stevea78 pushed a commit to stevea78/beats that referenced this pull request May 20, 2018
…les (elastic#6941)

Fixes elastic#5512 and elastic#6196

I've also changed the metricbeat getting started to use asciidoc attributes to resolve "metricbeat" and "Metricbeat". My goal is to convert all the topics over to using attributes, but I don't want to open a big PR for those changes because it's easier for me to check for mistakes if I make the changes in smaller batches.
dedemorton added a commit to dedemorton/beats that referenced this pull request Jul 24, 2018
…les (elastic#6941)

Fixes elastic#5512 and elastic#6196

I've also changed the metricbeat getting started to use asciidoc attributes to resolve "metricbeat" and "Metricbeat". My goal is to convert all the topics over to using attributes, but I don't want to open a big PR for those changes because it's easier for me to check for mistakes if I make the changes in smaller batches.
ruflin pushed a commit that referenced this pull request Jul 24, 2018
…les (#6941) (#7686)

Fixes #5512 and #6196

I've also changed the metricbeat getting started to use asciidoc attributes to resolve "metricbeat" and "Metricbeat". My goal is to convert all the topics over to using attributes, but I don't want to open a big PR for those changes because it's easier for me to check for mistakes if I make the changes in smaller batches.
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…les (elastic#6941) (elastic#7686)

Fixes elastic#5512 and elastic#6196

I've also changed the metricbeat getting started to use asciidoc attributes to resolve "metricbeat" and "Metricbeat". My goal is to convert all the topics over to using attributes, but I don't want to open a big PR for those changes because it's easier for me to check for mistakes if I make the changes in smaller batches.
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.

Update metricbeat getting started to show how to use modules.d layout
3 participants