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

Update info about loading dashboards and index templates #4778

Merged
merged 8 commits into from Aug 2, 2017

Conversation

dedemorton
Copy link
Contributor

@dedemorton dedemorton commented Jul 27, 2017

I plan to make additional changes to the getting started flow about setting up the dashboards because the current flow is not ideal, but I'll make that in a separate PR.

UPDATE: Changed my mind. Didn't want to have two separate PRs with a mix of changes, so I'll put everything in this PR.

@dedemorton dedemorton added docs review needs_backport PR is waiting to be backported to other branches. labels Jul 27, 2017
[[ulimit]]
=== {beatname_uc} fails to watch folders because too many files are open?

Because of the way file monitoring is implemented on Mac OS, you may see a
Copy link
Member

Choose a reason for hiding this comment

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

Apple refers to it as "macOS". https://www.apple.com/macos/sierra/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

@dedemorton
Copy link
Contributor Author

@andrewkroh I'm working on another set of changes related to dasbhboard loading. There's quite a bit of overlap with this PR, so I'm going to take the changes in my other branch and cherry-pick them into this branch. I'll mark this PR as "in progress" and continue adding commits to it. I'm going to have to make changes to multiple beats to avoid breaking things, so I'll change the name of this PR, too.

@dedemorton dedemorton added the in progress Pull request is currently in progress. label Jul 29, 2017
@dedemorton dedemorton changed the title Update Auditbeat docs with changes from testing Update info about loading dashboards and index templates Jul 29, 2017

//REVIEWERS: I'm keeping separate steps for the loading the index template and the dashboards since it seems like the index template loading happens automatically, but the dashboard loading must be specified. It might be confusing to describe this in one step. Plus having separate steps helps users understand that two very different operations are taking place (the loading of the index template into ES and the loading of dashboards into Kibana)

//TODO: I think this needs to be deleted. Commenting it out for now just in case. Also need to remove conditional flag
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can someone confirm that there is no reason to keep the content commented out here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's perfect the way it's described.

is false.

*`settings.index`*:: A dictionary of settings to place into the `settings.index` dictionary of the
//REVIEWERS: I don't see the following options in the reference.yml file. Are they still available/valid?
Copy link
Contributor

Choose a reason for hiding this comment

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

The setup.template.settings shows up only in the metricbeat.yml, not in the reference.yml. I will fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch @dedemorton. Open a PR: #4791

{libbeat}/config-file-permissions.html[Config File Ownership and Permissions]
in the _Beats Platform Reference_).

//REVIEWERS: It seems really inconvenient to users to expect them to change ownership on all the files in `modules.d`. This is something I had to do, but not 100% sure if there is a better way. Please advise.

Copy link
Contributor

Choose a reason for hiding this comment

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

@exekias can have a better input here.

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 noticed that Tudor also created an issue about this, so I think it is required.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree we may need to think about something here, we have strict.perms flag to avoid this (with the cost of losing some security). deb/rpm packages also ave these permissions right by default. Not sure we can help on the tar.gz case

Copy link
Member

Choose a reason for hiding this comment

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

I think using the strict.perms is fine here. I'd explain why it's used and link over to docs for the option.

I made this comment previously, and I still think we should consider it.

Another option would be to make -strict.perms=false the new default. Then modify the scripts that start Beats as services to enable the checks. I liken this to how ES disables bootstrap checks for non-production mode.

Copy link
Contributor Author

@dedemorton dedemorton Aug 1, 2017

Choose a reason for hiding this comment

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

@andrewkroh Ok, I've added the note about -strict.perms. I know we're talking about the getting started experience here, but I sort of worry if we start recommending something in the GS that could get people into trouble later on. See how this works. If the developers on the team decide that we want to tell users straight out to run the the beats with -strict.perms=false then I can change it, but the way it's written now users at least understand that there's a choice to be made...and might do more reading before they run the command blindly. (People often run commands without reading the surrounding text.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be happy to make -strict.perms=false the default in code when we detect "dev" mode or "getting started". The question is, what's a good sign of production? Is the fact you are running from init script / service file enough? Is deb/rpm production and the tar.gz getting started?

@@ -289,7 +297,7 @@ docker run {dockerimage}
[source,shell]
----------------------------------------------------------------------
sudo chown root packetbeat.yml <1>
sudo ./packetbeat -e -c packetbeat.yml -d "publish"
sudo ./packetbeat run -e -c packetbeat.yml -d "publish"
Copy link
Contributor

Choose a reason for hiding this comment

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

run is the default option, in case it's missing. So, it still works:

sudo ./packetbeat -e -c packetbeat.yml -d "publish"

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 Right, but I figured we added run for a reason and might want to show it. Do you think it's better to show the command without including run?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's better to show the command without including run to show that the command didn't change.

You can specify the following options in the `setup.kibana` section of the
+{beatname_lc}.yml+ config file:

//REVIEWERS: I've decided to use full paths when referring to options rather than using indentation in examples. WDYT? Makes the examples a tad harder to read but will help immensely with users who don't know where to put stuff in the yaml hierarchy.
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea. This is easier for the users to copy & paste to their configuration file.

[float]
==== `setup.kibana.host`

The Kibana host where the dashboards will be loaded. This setting is required.
Copy link
Contributor

Choose a reason for hiding this comment

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

By default is 127.0.0.1:5601.

The Kibana host where the dashboards will be loaded. This setting is required.
The scheme and port default to `http` and `5601` if you don't set them
explicitly. If you use a path after the port number, you must specify
the scheme and port. For example: `http://localhost:5601/path`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The Kibana configuration is the same as the Elasticsearch output configuration (https://www.elastic.co/guide/en/beats/filebeat/6.x/elasticsearch-output.html#hosts-option).
The host can be an URL or IP:PORT. For example: http://192.15.3.2, 192:15.3.2:5601, http://192.15.3.2:6701/path. If no port is specified, then 5601 is used.

Note: When a node is defined as an IP:PORT, the scheme and path are taken from the protocol and path config options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

`https`. The default is `http`. However, if you specify a URL for host, the
value of `protocol` is overridden by whatever scheme you specify in the URL.

//REVIEWERS: Please check this example. I have not tested it.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's right.


Enables {beatname_uc} to use SSL settings when connecting to Kibana via HTTPS.

//REVIEWERS: The reference config file said that the ssl.enabled: true is the default, but that doesn't seem quite right. Do you mean that this setting defaults to true when https is configured?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, if the user configures https then the default ssl values are used.

setup.kibana.ssl.certificate_authorities: ["/etc/pki/root/ca.pem"]
setup.kibana.ssl.certificate: "/etc/pki/client/cert.pem"
setup.kibana.ssl.key: "/etc/pki/client/cert.key
----
Copy link
Contributor

Choose a reason for hiding this comment

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

seems ok to me

[float]
==== `setup.kibana.ssl.verification_mode`

//REVEIWERS: Most of this content is copied from the section about configuring outputs. We should probably centralize our descriptions of these options at some point, but copy/paste is faster, so I'm doing that for now
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, ideally would be not to copy it from here: https://www.elastic.co/guide/en/beats/filebeat/6.x/configuration-output-ssl.html. In case something changes, we can change it in a single place.

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 If the descriptions truly are identical, then I should just point to the section. I need to:

  1. bump the topic about setting SSL options up one level in the nav so it's not nested under Configuring the output
  2. Add an explanation of when the SSL options are valid (output config and Kibana endpoint config).
  3. Rename the topic ID so it's not specific to configuring the output

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, the same ssl configuration is used also by logstash output, kafka output, so it's worth having a single copy and only point to it when needed.

@monicasarbu
Copy link
Contributor

Excellent job @dedemorton!

@dedemorton dedemorton removed the in progress Pull request is currently in progress. label Aug 1, 2017

Start Filebeat by issuing the appropriate command for your platform.

NOTE: If you use an init.d script to start Filebeat on deb or rpm, you can't
specify command line flags (see <<command-line-options>>). To specify flags,
start Filebeat in the foreground.

//REVIEWERS: With the new command syntax, should the deb and rpm instructions here say run instead of start?
Copy link
Member

Choose a reason for hiding this comment

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

No, for deb/rpm these commands remain the same. However for different reason we should change the commands to sudo service filebeat start. This ensures that service is started using a clean environment.

@dedemorton dedemorton closed this Aug 1, 2017
@dedemorton dedemorton reopened this Aug 1, 2017
@dedemorton
Copy link
Contributor Author

Sorry didn't mean to close the PR. Meant to cancel a comment that was in progress, but managed to close the PR and submit the comment instead. :-)

@andrewkroh
Copy link
Member

@dedemorton I saw this in a GH email but can't find the comment here.

@andrewkroh I think I accidentally blew away your comment about using -strict.perms=false. So are you suggesting that I add this flag to all the commands

If the packaging was tar.gz and the command uses sudo then I would add that flag rather than doing a chown.

@dedemorton
Copy link
Contributor Author

@dedemorton I saw this in a GH email but can't find the comment here.

Yah, sorry to waste your time. I meant to delete the comment, not submit it. I was going to make the change and then ask you to review it. I thought about the flag a little more. Since we recommend that users not use the flag, I think it's actually better to show the file ownership change and then modify the note to indicate that the users can run the beat with -strict.perms=false. That way someone won't say "but the example in the Getting Started said to use the flag." I just made the changes and will push them so you can review.

@dedemorton
Copy link
Contributor Author

@monicasarbu I think this should be close to being ready to merge, no?

@monicasarbu
Copy link
Contributor

Yes, it's ready to be merged.

@dedemorton dedemorton merged commit 1946fe0 into elastic:master Aug 2, 2017
dedemorton added a commit to dedemorton/beats that referenced this pull request Aug 2, 2017
Update Auditbeat docs with changes from testing
@dedemorton dedemorton removed the needs_backport PR is waiting to be backported to other branches. label Aug 2, 2017
andrewkroh pushed a commit that referenced this pull request Aug 2, 2017
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
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.

None yet

5 participants