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

Use import/export API for loading dashboards and index pattern #4413

Merged
merged 2 commits into from
Jul 3, 2017

Conversation

monicasarbu
Copy link
Contributor

@monicasarbu monicasarbu commented May 29, 2017

This is trying to implement #4409.

The Beats 6.x dashboards are different than the Beats 5.x dashboards and are not compatible. Thanks to the Kibana API, we can have a json file for each module instead of multiple json files (for dashboards, visualizations, searches, etc). Thus, the way to import/export dashboards differs from the Kibana version that should be equal to the Elasticsearch version.

The json file containing the index-pattern contain as version the Beat version, and the dashboards contain as version the Kibana version that exported the dashboards.

If Kibana version is 5.x and older:

  • The dashboards should be imported/exported directly from Elasticsearch, and it works only if Elasticsearch is set as output
  • All the Beats dashboards are moved to the _meta/kibana/5.x directory

If Kibana version is 6.x and greater:

  • The dashboards should be imported/exported via the Kibana API, so the user needs to configure the kibana url under the setup.kibana* settings.
  • All the Beats 6.x dashboards should be added to the _meta/kibana/default directory for each Beat module.

In order to make use of the import/export API the following changes are needed:

  • move all the old dashboards, search, visualizations, index-pattern to _meta/kibana/5.x directory
    and create a new directory under _meta/kibana/default for the new dashboards and index-pattern that are importable via the Kibana API
  • update generate_index_pattern to generate the index-pattern for 5.x and >= 6.x versions
  • use the Kibana API to load the dashboards and the index pattern for Kibana >= 6
  • update import_dashboards script
  • update make update
  • update integration tests

Note: The older dashboards are moved to the _meta/kibana/5.x directory only for Metricbeat for now.

@monicasarbu monicasarbu added the in progress Pull request is currently in progress. label May 29, 2017
@ruflin
Copy link
Contributor

ruflin commented May 30, 2017

@monicasarbu Should this also go under dev-tools/cmd/* like all the other commands?

@monicasarbu monicasarbu changed the title Use import/export API for loading dashboards and index pattern [WIP] Use import/export API for loading dashboards and index pattern Jun 1, 2017
@monicasarbu
Copy link
Contributor Author

@ruflin This PR is at a very early stage. My plan is to move the import and export functionality to libbeat and then create a script to use it. For now it's just a script (that I will remove) only for testing the import/export API.

@monicasarbu monicasarbu force-pushed the use_import_export_api branch from e85f3e2 to 8e271db Compare June 12, 2017 11:20
@monicasarbu monicasarbu force-pushed the use_import_export_api branch 2 times, most recently from 9160e6f to a294e57 Compare June 24, 2017 07:07
@monicasarbu monicasarbu changed the title [WIP] Use import/export API for loading dashboards and index pattern Use import/export API for loading dashboards and index pattern Jun 24, 2017
@monicasarbu monicasarbu force-pushed the use_import_export_api branch 5 times, most recently from 0efa466 to 9446eff Compare June 28, 2017 11:48
@tsg
Copy link
Contributor

tsg commented Jun 28, 2017

I think we need to add setup.kibana to the short config as well, using localhost by default.

@monicasarbu
Copy link
Contributor Author

@tsg Fixed, thanks!

@monicasarbu monicasarbu removed the in progress Pull request is currently in progress. label Jun 28, 2017
return fmt.Errorf("fail to create the Kibana loader: %v", err)
}

if kibanaLoader == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that etiher err == nil or kibanaLoader != nil, so I think the second check here is not needed and it doesn't follow our usual error checking pattern.

}

func (loader KibanaLoader) Close() error {
//return loader.client.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that there's nothing to do on Close. Then you can just delete the comment, not to confuse future readers.

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

logp.Debug("kibana", "Kibana url: %s", kibanaURL)

client := &Client{

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor issue, but this extra empty line could be deleted.

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

URL: kibanaURL,
http: &http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // ignore expired SSL certificates
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, does this mean that HTTPS is always enabled? The comment looks like "insecure by default", we have to be careful with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something that we need to address in the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to the TODO list: #4409


if cfg == nil {
cfg = common.NewConfig()
err := cfg.SetBool("enabled", -1, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

"enabled" is forced to true here, but I didn't see where it's actually checked. We should make sure that disabling with enabled: false works.

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


kibanaURL, err := common.MakeURL(config.Protocol, config.Path, config.Host, 5601)
if err != nil {
logp.Err("Invalid host param set: %s, Error: %v", config.Host, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

We generally avoid logp.Err immediately followed by returning an error.

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


reqURL, err := addPath(conn.URL, extraPath)
if err != nil {
return 0, nil, fmt.Errorf("fail to parse URL %s: %v", conn.URL, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be perhaps better to write the "fail to parse URL" error in the addPath method?

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


req, err := http.NewRequest(method, reqURL, body)
if err != nil {
logp.Err("Fail to create a HTTP %s request: %v", method, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Return error instead of logging.

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


resp, err := conn.http.Do(req)
if err != nil {
logp.Err("Fail to execute a HTTP %s request: %v", method, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

same 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.

fixed


_, result, err := client.Connection.Request("GET", "/api/status", nil, nil)
if err != nil {
return "", fmt.Errorf("HTTP GET request to /api/status fails: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the result to the error message.

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

var kibanaVersion kibanaVersionResponse
err = json.Unmarshal(result, &kibanaVersion)
if err != nil {
return "", fmt.Errorf("fail to unmarshall the HTTP response from Kibana %s: %v", client.Connection.URL, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

unmarshal with one l.

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

if err != nil {
return fmt.Errorf("%v. Response: %s", err, response)
}
if statusCode != 200 {
Copy link
Contributor

Choose a reason for hiding this comment

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

200 might be too strict here. Maybe > 300?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, fixed

)

var importAPI = "/api/kibana/dashboards/import"
var exportAPI = "/api/kibana/dashboards/export"
Copy link
Contributor

Choose a reason for hiding this comment

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

exportAPI doesn't seem to be used.

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

@@ -305,16 +306,8 @@ index-template: ## @build Generate index templates for the given $VERSION. This
### KIBANA FILES HANDLING ###
ES_URL?=http://localhost:9200

.PHONY: export-dashboards
export-dashboards: python-env update
. ${PYTHON_ENV}/bin/activate && python ${ES_BEATS}/dev-tools/export_dashboards.py --url ${ES_URL} --dir $(shell pwd)/_meta/kibana --regex ${BEAT_NAME}-*
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we get export-dashboards and import-dashboards targets based on the go code? I'm good with follow up PR for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit tricky for export-dashboards as we need to pass the list of ids that we want to export in one json file. Usually you would run it to export the dashboards for a module, not for a Beat. Added to the TODO list in the meta issue: #4409

@monicasarbu monicasarbu force-pushed the use_import_export_api branch 2 times, most recently from 00523db to 6139a64 Compare June 29, 2017 13:04
@exekias
Copy link
Contributor

exekias commented Jun 29, 2017

I think it would make sense to add dashboards to the existing export command, I think users could use that to share easily their own dashboards

Copy link
Contributor

@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.

If we update / improve dashboards for 6.x in the future, will we also update 5.x dashboards? Probably best would be that we "freeze" them as they are and improvements / new features only are available for 6.x?

.gitignore Outdated
@@ -22,6 +22,9 @@ coverage.out
*.swo
*.swn

# Executable
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 move this to a local ignore file in 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.

Good idea, done.

# Scheme and port can be left out and will be set to the default (http and 5601)
# In case you specify and additional path, the scheme is required: http://localhost:5601/path
# IPv6 addresses should always be defined as: https://[2001:db8::1]:5601
# host: "localhost:5601"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove spaces in front of config options for easier uncommenting. Also below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

}
func getMajorVersion(version string) (int, error) {

majorVersion := string(version[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

This will break with release 10.* :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@monicasarbu monicasarbu force-pushed the use_import_export_api branch from a5b8147 to 1496049 Compare July 3, 2017 08:19
@monicasarbu
Copy link
Contributor Author

@ruflin yes, that's the plan to freeze the 5.x dashboards and improve only the 6.x dashboards.

@monicasarbu monicasarbu force-pushed the use_import_export_api branch from 218c11b to 57d4a31 Compare July 3, 2017 09:46
If the Elasticsearch version is older than 6.x, then the dashboards are loaded directly into Elasticsearch. The old
dashboards are moved to the 5.x directory.
@tsg
Copy link
Contributor

tsg commented Jul 3, 2017

jenkins, test it

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.

4 participants