-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Metricbeat] Add STAN dashboard for module #15654
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you need to run make update
:
Error: some files are not up-to-date. Run 'mage fmt update' then review and commit the changes. Modified: [metricbeat/docs/modules/stan.asciidoc metricbeat/docs/modules_list.asciidoc]
metricbeat/include/list_common.go
Outdated
@@ -128,6 +128,7 @@ import ( | |||
_ "github.com/elastic/beats/metricbeat/module/redis/info" | |||
_ "github.com/elastic/beats/metricbeat/module/redis/key" | |||
_ "github.com/elastic/beats/metricbeat/module/redis/keyspace" | |||
_ "github.com/elastic/beats/metricbeat/module/stan" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this shouldn't be here. It will be fixed with a make update
command inside /beats/metricbeat
folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure why running make check
in my local repo doesn't cause this error though while Travis complains? Been driving me nuts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make update
adds it back after I removed it and ran locally. Something is out of sync and even merging the PR into a local copy of master
and running these updates don't reproduce the error for myself either.
ca608ea
to
ada055c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for resolving the problem. I just left some minor suggestions regarding the naming.
Also could you please update the https://github.com/elastic/beats/blob/10c741a8d8a16de9ee317887fcf90036fa0fd214/x-pack/metricbeat/module/stan/module.yml accordingly with the proper filename and dashobard ID?
Last but not least a changelog entry would be needed (
Line 95 in 3dd38c1
- [Metricbeat][Istio] Add mixer metricset {pull}15696[15696] |
x-pack/metricbeat/module/stan/_meta/kibana/7/Metricbeat-STAN-overview.json
Outdated
Show resolved
Hide resolved
x-pack/metricbeat/module/stan/_meta/kibana/7/Metricbeat-STAN-overview.json
Outdated
Show resolved
Hide resolved
x-pack/metricbeat/module/stan/_meta/kibana/7/Metricbeat-STAN-overview.json
Outdated
Show resolved
Hide resolved
x-pack/metricbeat/module/stan/_meta/kibana/7/Metricbeat-STAN-overview.json
Outdated
Show resolved
Hide resolved
x-pack/metricbeat/module/stan/_meta/kibana/7/Metricbeat-STAN-overview.json
Outdated
Show resolved
Hide resolved
edb4db7
to
249db3f
Compare
…verview.json Co-Authored-By: Chris Mark <chrismarkou92@gmail.com>
…verview.json Co-Authored-By: Chris Mark <chrismarkou92@gmail.com>
…verview.json Co-Authored-By: Chris Mark <chrismarkou92@gmail.com>
945b79b
to
ac050f6
Compare
ac050f6
to
129b9e8
Compare
metricbeat/docs/fields.asciidoc
Outdated
@@ -31244,7 +31244,6 @@ type: object | |||
stan Module | |||
|
|||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not part of this PR right? Could we clean this?
metricbeat/module/stan/fields.go
Outdated
@@ -0,0 +1,36 @@ | |||
// Licensed to Elasticsearch B.V. under one or more contributor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this file should be here? Is there any reason for adding this? I think we should clean this up.
@@ -1,7 +1,6 @@ | |||
- key: stan | |||
title: "Stan" | |||
description: > | |||
stan Module | |||
description: stan Module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change needed?
@devon-kim I tried to fix the problems with We will need an approval from @fearful-symmetry so as to enable merging. |
Tests are still failing:
|
ouch, thanks for noticing! I changed the problematic objects to an older version, let's see if this helps. Maybe we need to update our CI though? |
@fearful-symmetry CI looks good now. |
(cherry picked from commit e3588da)
(cherry picked from commit e3588da)
Some issues with the dashboard found that will be fixed by #15960. |
Tested this with BC5 and works as expected after changes applied on #15960 |
What does this PR do?
Add Kibana dashboards for the Metricbeat STAN module
Why is it important?
NATS module already has a dashboard, STAN module should have one as well for consistency in support
Checklist
Author's Checklist
Related issues
Screenshots
Logs
N/A