Skip to content

Suppress dashboard info when dashboards aren't available#8395

Merged
dedemorton merged 1 commit intoelastic:masterfrom
dedemorton:issue#8381
Sep 28, 2018
Merged

Suppress dashboard info when dashboards aren't available#8395
dedemorton merged 1 commit intoelastic:masterfrom
dedemorton:issue#8381

Conversation

@dedemorton
Copy link
Copy Markdown
Contributor

Closes #8381

With these changes in place, dashboard-related content will be suppressed for Filebeat modules that do not include dashboards. I also did some cleanup work because attributes should be reset when their values are no longer needed.

Implementation note:
It would have been simpler to implement this as an ifndef but I was concerned that new module contributors would overlook the need to add the attribute. Having an attribute defined at the top of the file makes it more obvious that there's something to define, even if the module developer inadvertently removes the dashboard section without reading it first.

@dedemorton dedemorton added docs review needs_backport PR is waiting to be backported to other branches. labels Sep 21, 2018
@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Sep 24, 2018

We can automatically detect if a module has a dashboard or not and do it for the overview page. I wonder if we could do something similar here so it gets automatically updated and we don't have to apply it manually?

@dedemorton
Copy link
Copy Markdown
Contributor Author

dedemorton commented Sep 24, 2018

@ruflin I knew we could do that for Metricbeat, but wasn't sure about Filebeat. We don't currently show dashboards on the Filebeat overview page.

We'd need to set the has-dashboards attribute programmatically to get the conditional coding in the shared files to work. We could do that in a separate PR. To make the logic easier, maybe it would be better to have has-dashboards as the first attribute on the page?

@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Sep 25, 2018

Correct, we don't do it for Filebeat at the moment (we probably could). As we don't have it yet, lets move forward with this manual solution and automated it later.

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

@dedemorton dedemorton merged commit 6ba1126 into elastic:master Sep 28, 2018
@dedemorton dedemorton removed the needs_backport PR is waiting to be backported to other branches. label Oct 1, 2018
@dedemorton dedemorton deleted the issue#8381 branch October 1, 2018 23:35
dedemorton added a commit to dedemorton/beats that referenced this pull request Oct 17, 2018
dedemorton added a commit that referenced this pull request Oct 18, 2018
…8478) (#8529)

* Clarify support for ssl options for modules (#7967)

* Clarify support for ssl options for modules

* Change example to show http module

* Update Elasticsearch module examples to show http in the URL (#8226)

* Improve reference docs that describe how to set options dynamically (#8290)

* Improve Elasticsearch output docs about indices, pipelines, and keys settings

* Updates from review

* Change setting name from mapping to mappings

* Remove note to reviewer

* Fix conditional coding (#8446)

* Suppress dashboard info when dashboards aren't available (#8395)

* Clarify add_docker_metadata docs (#8478)
dedemorton added a commit to dedemorton/beats that referenced this pull request Oct 18, 2018
dedemorton added a commit that referenced this pull request Oct 19, 2018
…8478) (#8528)

* Clarify support for ssl options for modules (#7967)

* Clarify support for ssl options for modules

* Change example to show http module

* Update Elasticsearch module examples to show http in the URL (#8226)

* Improve reference docs that describe how to set options dynamically (#8290)

* Improve Elasticsearch output docs about indices, pipelines, and keys settings

* Updates from review

* Change setting name from mapping to mappings

* Remove note to reviewer

* Fix conditional coding (#8446)

* Suppress dashboard info when dashboards aren't available (#8395)

* Clarify add_docker_metadata docs (#8478)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…tic#8290 elastic#8395 elastic#8446 elastic#8478) (elastic#8528)

* Clarify support for ssl options for modules (elastic#7967)

* Clarify support for ssl options for modules

* Change example to show http module

* Update Elasticsearch module examples to show http in the URL (elastic#8226)

* Improve reference docs that describe how to set options dynamically (elastic#8290)

* Improve Elasticsearch output docs about indices, pipelines, and keys settings

* Updates from review

* Change setting name from mapping to mappings

* Remove note to reviewer

* Fix conditional coding (elastic#8446)

* Suppress dashboard info when dashboards aren't available (elastic#8395)

* Clarify add_docker_metadata docs (elastic#8478)
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.

3 participants