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

Unify dashboard exporter tools #9097

Merged
merged 15 commits into from
Nov 27, 2018

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Nov 15, 2018

The existing export_dashboards.go and the command export dashboards are unified, using the same code and slightly different logic. Configuration of export_dashboards is done using the following command line options: -kibana, -space, -output, -quiet and -index-pattern. The last flag is an odd one out, because it is part of the script, but its value is never used. So far no one complained, so I did not port it. The default value of that flag is the same in case of both methods.

Configuration the Kibana client of export dashboard is read from the config of the Beat. Thus, Kibana-related flags are not part of its CLI.

By default export dashboard does not decode the exported dashboard. If flag -decode passed to the command, the dashboard is decoded.

Equivalent commands

export dashboards from a dashboards.yml

$ ./filebeat export dashboard -yml path/to/dashboards.yml -decode
$ go run dev-tools/cmd/dashboards/export_dashboards.go -yml path/to/dashboards.yml

export a dashboard with an id and print to stdout

$ ./filebeat export dashboard -id {uuid} -decode
$ go run dev-tools/cmd/dashboards/export_dashboards.go -dashboard {uuid}

The interface of export dashboard is different than in the previous PR, because I wanted to follow existing methods and documentation.

TODO

  • add automated tests
  • add new export dashboard option to dev docs

@kvch kvch added in progress Pull request is currently in progress. refactoring libbeat labels Nov 15, 2018
@kvch kvch requested review from ph and urso November 15, 2018 13:51
libbeat/dashboards/export.go Outdated Show resolved Hide resolved
libbeat/dashboards/export.go Show resolved Hide resolved
libbeat/dashboards/export.go Outdated Show resolved Hide resolved
libbeat/dashboards/export.go Outdated Show resolved Hide resolved
@kvch kvch added review needs_backport PR is waiting to be backported to other branches. labels Nov 15, 2018
@kvch kvch removed the in progress Pull request is currently in progress. label Nov 16, 2018
libbeat/kibana/client.go Outdated Show resolved Hide resolved
@@ -213,6 +213,8 @@ https://github.com/elastic/beats/tree/master/dev-tools/cmd/dashboards[dev-tools]
for exporting Kibana 5.x dashboards. See the dev-tools
https://github.com/elastic/beats/tree/master/dev-tools/README.md[readme] for more info.

Alternatively, if the scripts above are not available, you can use your Beat binary to export Kibana 6.0 dashboards or later.
Copy link
Member

Choose a reason for hiding this comment

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

Will this PR be backported? If not, we could remove everything related to 5.x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is planned to as the label suggests.

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

good work @kvch and thanks for adding integration tests. I have added a few minors things.

I see we test for happy path in the integration tests could we add tests for non-happy-path like when dashboard don't exist?

dev-tools/cmd/dashboards/export_dashboards.go Outdated Show resolved Hide resolved
dev-tools/cmd/dashboards/export_dashboards.go Outdated Show resolved Hide resolved
libbeat/cmd/export/dashboard.go Outdated Show resolved Hide resolved
libbeat/dashboards/export.go Outdated Show resolved Hide resolved
libbeat/dashboards/export.go Outdated Show resolved Hide resolved
libbeat/kibana/client.go Outdated Show resolved Hide resolved
libbeat/kibana/client.go Outdated Show resolved Hide resolved
libbeat/dashboards/export.go Show resolved Hide resolved
@kvch
Copy link
Contributor Author

kvch commented Nov 21, 2018

I added more E2E tests which cover requesting unknown dashboards and invalid files.

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

Added a note about my mistake in parsing URL :(

TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // ignore expired SSL certificates
u, err := url.Parse(*kibanaURL)
if err != nil {
log.Fatalf("Error parsing Kibana URL: %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.

I have a mistake, url.Parse or url.ParseRequestURL is well really bad see for yourself:

https://play.golang.org/p/9NAU0-sasfT

I think we have to come up with something better, maybe we already have in other outputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In elasticsearch output url.Parse is used. Looking at the codebase, everytime we connect to something url.Parse is used. I think it is good enough.

@ruflin
Copy link
Member

ruflin commented Nov 23, 2018

If I remember correctly the beat export dashboard command did export the dashboards without decoding to make sure they can be directly imported again. The dev export command also did the decoding to make it better for versioning. I skimmed through the code and it seems now in both cases the decoding happens? Didn't test locally.

@kvch
Copy link
Contributor Author

kvch commented Nov 23, 2018

@ruflin That's exactly how it worked. I added the option -decode to export dashboard, so users can decide if they want a decoded version or not. I left the functionality of export_dashboard.go as is.

@ruflin
Copy link
Member

ruflin commented Nov 23, 2018

@kvch This is great. Didn't get to review the code itself yet but functionality is exactly what I hoped for. So if @ph is 👍 I'm good too :-D

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

👍 LGTM, I've looked at the CI this is a metricbeat/kafka docker issue.. nothing that this PR introduce.

@kvch kvch force-pushed the refactoring-libbeat-unify-export-dashboards branch from 6b096de to ee19147 Compare November 26, 2018 14:55
@kvch
Copy link
Contributor Author

kvch commented Nov 26, 2018

Added forgotten changelog entry.

@kvch kvch merged commit 1411852 into elastic:master Nov 27, 2018
@kvch kvch added v6.6.0 and removed needs_backport PR is waiting to be backported to other branches. labels Nov 27, 2018
kvch added a commit to kvch/beats that referenced this pull request Nov 27, 2018
The existing `export_dashboards.go` and the command `export dashboards` are unified, using the same code and slightly different logic. Configuration of `export_dashboards` is done using the following command line options: `-kibana`, `-space`, `-output`, `-quiet` and `-index-pattern`. The last flag is an odd one out, because it is part of the script, but its value is never used. So far no one complained, so I did not port it. The default value of that flag is the same in case of both methods.

Configuration the Kibana client of `export dashboard` is read from the config of the Beat. Thus, Kibana-related flags are not part of its CLI.

By default `export dashboard` does not decode the exported dashboard. If flag `-decode` passed to the command, the dashboard is decoded.

export dashboards from a dashboards.yml
```
$ ./filebeat export dashboard -yml path/to/dashboards.yml -decode
$ go run dev-tools/cmd/dashboards/export_dashboards.go -yml path/to/dashboards.yml
```
export a dashboard with an id and print to stdout
```
$ ./filebeat export dashboard -id {uuid} -decode
$ go run dev-tools/cmd/dashboards/export_dashboards.go -dashboard {uuid}
```
(cherry picked from commit 1411852)
kvch added a commit that referenced this pull request Nov 27, 2018
The existing `export_dashboards.go` and the command `export dashboards` are unified, using the same code and slightly different logic. Configuration of `export_dashboards` is done using the following command line options: `-kibana`, `-space`, `-output`, `-quiet` and `-index-pattern`. The last flag is an odd one out, because it is part of the script, but its value is never used. So far no one complained, so I did not port it. The default value of that flag is the same in case of both methods.

Configuration the Kibana client of `export dashboard` is read from the config of the Beat. Thus, Kibana-related flags are not part of its CLI.

By default `export dashboard` does not decode the exported dashboard. If flag `-decode` passed to the command, the dashboard is decoded.

export dashboards from a dashboards.yml
```
$ ./filebeat export dashboard -yml path/to/dashboards.yml -decode
$ go run dev-tools/cmd/dashboards/export_dashboards.go -yml path/to/dashboards.yml
```
export a dashboard with an id and print to stdout
```
$ ./filebeat export dashboard -id {uuid} -decode
$ go run dev-tools/cmd/dashboards/export_dashboards.go -dashboard {uuid}
```
(cherry picked from commit 1411852)
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.

5 participants