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

Make -setup load the Beat dashboards #3506

Merged
merged 1 commit into from
Feb 3, 2017

Conversation

tsg
Copy link
Contributor

@tsg tsg commented Jan 31, 2017

With this change, filebeat -setup loads the sample Kibana dashboards,
being the rough equivalent of running: scripts/import_dashboards && filebeat.
This works for all Beats, not only Filebeat. The import_dashboards program
is kept for now, to avoid braking compatibility. Code wise, the two share
most of the code, which was separated in a sub-package.

All flags available for the import_dashboard are available as go-ucfg options.
For example, to load the dashboards from a directory you can do:

filebeat -e -setup -E "dashboards.dir=_meta/kibana"

Or do load the snapshot version of the dashboards:

filebeat -e -setup -E "dashboards.snapshot=true"

This also changes the pipeline loading to happen automatically regardless of
whether -setup was used or not.

Part of #3159.

TODOs:

  • Add tests for the dashboard sub-package
  • Document the -setup flag and dashboard settings
  • Changelog + deprecation notice for the import_dashboard program (?)

@tsg tsg added in progress Pull request is currently in progress. review labels Jan 31, 2017
@tsg tsg requested a review from monicasarbu January 31, 2017 23:12
@tsg tsg mentioned this pull request Jan 31, 2017
22 tasks
@tsg tsg force-pushed the filebeat_modules_dashboards branch 2 times, most recently from 164700a to 0d8d8b6 Compare February 2, 2017 11:38
@tsg tsg requested a review from dedemorton February 2, 2017 11:39
@tsg
Copy link
Contributor Author

tsg commented Feb 2, 2017

@dedemorton I'd appreciate your review on the docs part.

#dashboards.file:

# If this option is enabled, the snapshot URL is used instead of the default URL.
#dashboard.snapshot: false
Copy link
Member

Choose a reason for hiding this comment

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

Nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a feature we already have in `import_dashboards" :)

@tsg tsg removed the in progress Pull request is currently in progress. label Feb 2, 2017
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

It's really great to have all dashboard config options now also directly in the config file and indirectly available through the -E flag.

Question for the -setup flag: If I run now ./filebeat -setup will filebeat stop after the setup phase or will it continue running? And will it load only dashboards or also pipelines, index patterns etc?

}

var (
printVersion = flag.Bool("version", false, "Print the version and exit")
setup = flag.Bool("setup", false, "Load the sample Kibana dashboards")
Copy link
Member

@ruflin ruflin Feb 2, 2017

Choose a reason for hiding this comment

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

Load sampel Kibana dashboards, index template, patterns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment it only loads the dashboards, see the other answer.

@@ -0,0 +1,535 @@
package dashboards
Copy link
Member

Choose a reason for hiding this comment

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

Why is it dashboards/dashboards/...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First dashboards is the current import_dashboards main package. The second is where I gathered most of the code. After we remove import_dashboards we can move the package one level up.

@@ -0,0 +1,294 @@
########################## Metricbeat Configuration ###########################
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't this under .gitignore? Do we need it again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, weird, i'll check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a typo in .gitignore, I deleted it again.

@tsg
Copy link
Contributor Author

tsg commented Feb 2, 2017

Question for the -setup flag: If I run now ./filebeat -setup will filebeat stop after the setup phase or will it continue running? And will it load only dashboards or also pipelines, index patterns etc?

It will continue in case of success and stop in case of errors. Since we still have the import_dashboards script that does only that, I thought it would be interesting to experiment with letting it run in case of success. So far I like it like this, but it's debatable.

At the moment, -setup loads strictly the dashboards. Everything else (templates, pipelines, etc.) is happening automatically always, since they are lightweight operations and the Beats mallfunction if they are not loaded. The dashboards, on the other hand, are relatively heavy to load and can also be loaded after starting the Beat, without any implication regarding the correctness of the data.

@ruflin
Copy link
Member

ruflin commented Feb 2, 2017

@tsg Thanks for the details. My preferred option would be as following:

  • -setup loads everything (dashboards, pipelines, templates, ...) and stops afterwards as this is run only on one machine / separate machine. This would mostly replace the dashboard script.
  • Without setup, template, pipelines are loaded by default, but can be disabled.

But if -setup only loads the dashboards, I can see that you're behaviour is preferrable as we also have "only" the script.

@tsg
Copy link
Contributor Author

tsg commented Feb 2, 2017

We could have both -setup and -setup-only as a compromise? The -setup is great for demos and getting started, since it removes one step. I didn't add -setup-only yet because we still have import_dashboards.

@ruflin
Copy link
Member

ruflin commented Feb 2, 2017

For demos what can be done now is ./filebeat -E dashboards.enabled:true which loads everything so not even -setup flag has to be used. That is the great thing about having it in the config file. Having this, -setup-only could be -setup.

@tsg
Copy link
Contributor Author

tsg commented Feb 2, 2017

We're splitting hairs now, but I think -setup would work better in demos & getting started than -E dashboards.enabled=true. We could have -E dashboards.stop_after_loading=true or something like that instead of -setup-only. In other words, flip when we need the -E.

Or maybe I'm wrong and we could skip both -setup and -setup-only and only rely on -E flags, this way we teach the user early on how to do that.

@tsg tsg force-pushed the filebeat_modules_dashboards branch from dd7f882 to f79a78d Compare February 2, 2017 22:06
#dashboards.index:

# If enabled, load only the dashboards and not the index pattern.
#dashboards.only_dashboards: false
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 it doesn't make sense to migrate these two options (only_dashboards and only_index) from import_dashboards, and leave -setup to import both dashboards and the index pattern.

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 removed the two options from the configuration files and docs, but they still exist in the code so that they keep working for import_dashboards. We can remove them from the code as well when we remove import_dashboards.

@tsg tsg force-pushed the filebeat_modules_dashboards branch from f79a78d to 90ee353 Compare February 2, 2017 22:36
With this change, `filebeat -setup` loads the sample Kibana dashboards,
being the rough equivalent of running: `scripts/import_dashboards && filebeat`.
This works for all Beats, not only Filebeat. The `import_dashboards` program
is kept for now, to avoid braking compatibility. Code wise, the two share
most of the code, which was separated in a sub-package.

All flags available for the `import_dashboard` are available as go-ucfg options.
For example, to load the dashboards from a directory you can do:

    filebeat -e -setup -E "dashboards.dir=_meta/kibana"

Or do load the snapshot version of the dashboards:

    filebeat -e -setup -E "dashboards.snapshot=true"

This also changes the pipeline loading to happen automatically regardless of
whether `-setup` was used or not.

Part of elastic#3159.

Contains updates for the docs and changelog.
@tsg tsg force-pushed the filebeat_modules_dashboards branch from 90ee353 to e10fbaa Compare February 2, 2017 22:53
@monicasarbu
Copy link
Contributor

monicasarbu commented Feb 2, 2017

I think what we should improve here on longer term is to remove the necessity of -setup to load the dashboards, and have each Beat check if the dashboards version X are loaded and load them only when needed. And maybe have a "dashboards.overwrite" boolean option. This will also solve the first experience of the user as only one Beat will be able to load the dashboard, when starting more Beats at the same time.

Another improvement that we can do is to load only the Kibana dashboards for which the modules are enabled, but this requires us to split the dashboards per module. The index pattern will be loaded for all modules.

For now, I would say to keep -setup import only the dashboards (and stop afterwards), and try to remove it with the time.

@dedemorton
Copy link
Contributor

@tsg sorry I didn't get to the review today (didn't see your request until late in the day). I'll look in the morning. But I have a couple of initial questions:

  1. Since this is Beta level, I'm assuming we'll leave the steps for loading the dashboards as they are in the getting started guides for now, right? At what point will the new way become the preferred way to install dashboards?
  2. What about the use case where community Beats developers need to import dashboards (documented here). Will we continue to provide the import_dashboards command for that specific use case?

@tsg
Copy link
Contributor Author

tsg commented Feb 3, 2017

Since this is Beta level, I'm assuming we'll leave the steps for loading the dashboards as they are in the getting started guides for now, right? At what point will the new way become the preferred way to install dashboards?

Correct, we don't have to change the getting started guide yet. I think the new way will become preferred in 6.0, but until then more changes might happen.

What about the use case where community Beats developers need to import dashboards (documented here). Will we continue to provide the import_dashboards command for that specific use case?

For the moment we keep the import_dashboards script, so we don't have to adjust the documentation. It'll also probably change with 6.0.

@monicasarbu monicasarbu merged commit 0d64737 into elastic:master Feb 3, 2017
@dedemorton dedemorton mentioned this pull request Feb 14, 2017
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants