Dashboard exporting saves to file in case of modules #9076
Conversation
kvch
added
in progress
| modulePath := filepath.Join(paths.Resolve(paths.Home, "module"), module) | ||
| stat, err := os.Stat(modulePath) | ||
| if err != nil || !stat.IsDir() { | ||
| return fmt.Errorf("no such module: %s\n", modulePath) |
There was a problem hiding this comment.
error strings should not be capitalized or end with punctuation or a newline
| ` | ||
| ) | ||
|
|
||
| func AddModuleDashboard(beatName, module, kibanaVersion, dashboardID string, dashboard common.MapStr, suffix string) error { |
There was a problem hiding this comment.
exported function AddModuleDashboard should have comment or be unexported
|
I like the proposal of this PR, concerning:
I think we should add logic to detect either that the id is already present in the module and if the file name is already there, if we don't the file will be in inconsistent state. Also, we can add a |
| return fmt.Errorf("no such module: %s\n", modulePath) | ||
| } | ||
|
|
||
| dashboardFile := strings.Title(beatName) + "-" + module + "-" + suffix + ".json" |
There was a problem hiding this comment.
Why 'title'? Given default case insensitivity Windows/macOS filesystems can be a pain with in comparison to Linux.
There was a problem hiding this comment.
I thought this format is the name convention for Filebeat module dashboards, as 99% percent of them are named Filebeat-{modulename}-{suffix}.json.
| f, err := os.OpenFile(filepath.Join(modulePath, "module.yml"), os.O_WRONLY|os.O_APPEND, 0644) | ||
| if err == nil { | ||
| _, err = f.Write([]byte(content)) | ||
| } |
There was a problem hiding this comment.
better use ioutil.WriteFile. This also checks the amount of bytes written and errors if write was incomplete.
There was a problem hiding this comment.
Oh... it's an append. In this case check the amount of bytes written. If some bytes are written, but write is incomplete then the file is broken.
| stat, err := os.Stat(modulePath) | ||
| if err != nil || !stat.IsDir() { | ||
| return fmt.Errorf("no such module: %s\n", modulePath) | ||
| } |
There was a problem hiding this comment.
If you want to report a more accurate error, I think you actually want to use https://golang.org/pkg/os/#IsExist instead of IsDir() or https://golang.org/pkg/os/#IsNotExist
There was a problem hiding this comment.
Why? If there is no error it means the file exists and I have the proper permission to access it. I also need to check if that module is a folder. If it is not a folder, I cannot create more folders under it and cannot saves files.
There was a problem hiding this comment.
I followed the same check as it is done in case of the ModuleRegistry: https://github.com/elastic/beats/blob/master/filebeat/fileset/modules.go#L116
| } | ||
| if version.Major < 6 { | ||
| return fmt.Errorf("saving exported dashboards is not available for Kibana version '%s'", version.String()) | ||
| } |
There was a problem hiding this comment.
It would be good to report that you need at least kibana 6.
| const ( | ||
| dashboardEntry = `- id: %s | ||
| file: %s | ||
| ` |
There was a problem hiding this comment.
Looking at the module.yml this looks like generic YAML files, minus the extra space between lines. Is there a specific reason why we don't use the YAML package and structs to serialize / unserialize content.
There was a problem hiding this comment.
No, nothing. I planned to use the yaml package to read and write module.yml. I was just curious if you like the direction of the PR. :)
|
Opening a new PR with the appropiate changes. Turns out I exported modules differently than the docs says. |
|
I like the concept of this PR. One suggestion: Instead of going with |
Based on #7506 (comment)
This PR adds two new flags to
export dashboardcommand:-module,-suffix.The flag
modulerequires an existing module name. If it is set the command creates a{modulename}/_meta/kibana/{kibanaversion}/dashboarddirectories if missing.{kibanaversion}is the version of the Kibana instance the dashboard is downloaded from. After the directories are created, the dashboard is saved as{Beatname}-{modulename}-{suffix}.json.-suffixis not required, by default it is set to"overview". Once the file is downloaded and saved, it is added to themodule.ymlof the module.So from now on the module generator does not create kibana directories. It was unnecessary at that point, because not all modules come with dashboards.
The following recipe is the current hybrid (for a time being) process to create new modules:
To export the dashboard the following command is called:
This creates the the required directories, downloads the dashboard, saves it in the required file and adds it to
modules.yml. Previously, all steps needed to be done manually except for the first two.This only supported from Kibana 6.x. If someone wants to export dashboard from an earlier version, he/she has to follow this docs: https://www.elastic.co/guide/en/beats/devguide/current/export-dashboards.html#_exporting_kibana_5_x_dashboards
The behaviour of
export dashboard -id {imagine-a-uuid-here}remains unchanged. Thus, no breaking changes are introduced.I added the flags to
libbeatto support all Beats which has similar module handling as Filebeat does. This will be also useful when modules/filesets of Filebeat are moved to libbeat.Limitations
module.ymlwhenever it is exported this way. There is no check if the UUID is unique in that file. A possible workaround is to delete all entries with the same UUID except for the last one. As it is the latest dashboard with that UUID.TODO