-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Conversation
+ | ||
["source","sh",subs="attributes"] | ||
---- | ||
docker run docker.elastic.co/beats/{beatname_lc}:{version} modules enable apache mysql |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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,
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
and the Apache HTTPD module: | ||
|
||
[source, shell] | ||
------------------------------------- |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Changes LGTM. Left one questions. |
…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.
…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.
…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.
…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.
…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.
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?