[Metricbeat] Release munin as GA. #10311
Conversation
ruflin
added
module
review
Metricbeat
ruflin
force-pushed
the
munin-ga
branch
2 times, most recently
from
4d533cf
to
2ea6045
Compare
| "uptime": 5.98 | ||
| }, | ||
| "users": { | ||
| "X": 0, |
There was a problem hiding this comment.
Maybe we should open an issue to lowercase this field... or rename it to something more meaningful (well... maybe it's super meaningful for people experienced in Munin)
There was a problem hiding this comment.
maybe it's super meaningful for people experienced in Munin
😆
| "forwarded": 0, | ||
| "received": 823 | ||
| }, | ||
| "if_err_eth0": { |
There was a problem hiding this comment.
All this fields seems dynamically generated from the machine network interfaces. I thought this wasn't ideal for a Metricbeat module. Just asking
There was a problem hiding this comment.
They are names of "plugins", now they are going to go to the munin.plugin.name field as values, so we avoid fields explosion. and problems of this kind.
I am open in the future to add some kind of grok-like patterns to extract fields from these names (e.g, extract labels with expressions like if_err_%{WORD:network_interface}).
|
Sorry, I overlooked #10322 and I can see that there's a lot of discussion around this module alreayd |
ruflin
removed
the
needs_backport
|
I removed |
Add integration tests and add data.json file.
metricbeat/module/munin/node/node.go
Outdated
| @@ -21,8 +21,11 @@ import ( | |||
| "time" | |||
|
|
|||
| "github.com/elastic/beats/libbeat/common" | |||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
expected 'STRING', found '<<' (and 10 more errors)
| "name": "host.example.com" | ||
| }, | ||
| "event": { | ||
| "dataset": "munin.node", |
There was a problem hiding this comment.
@jsoriano I'm thinking if the data set here should actually be munin.metrics. Same would apply to prometheus. WDYT?
There was a problem hiding this comment.
What I like of munin.node is that it refers to being a metricset that collects metrics from a munin node. But I don't have an strong opinion, feel free to change it, now that there are going to be so many breaking changes it would be the moment.
There was a problem hiding this comment.
To be honest I'm on the fence here. I like the name of the metricset itself and would keep it. I'm only thinking about the dataset name.
| @@ -268,6 +268,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d | |||
| - Release kvm module as beta. {pull}10279[10279] | |||
| - Release http.server metricset as GA. {pull}10240[10240] | |||
| - Release Nats module as GA. {pull}10281[10281] | |||
| - Release munin module as GA. {pull}10311[10311] | |||
There was a problem hiding this comment.
I am still not 100% sure that current implementation is a final one, but I feel more confident that at least the settings and the field mappings are better than before.
There was a problem hiding this comment.
We can still improve it moving forward but keep compatibilty.
Add integration tests and add data.json file.