Add metrics metricset to MongoDB module#7611
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
jsoriano
left a comment
There was a problem hiding this comment.
Only some small comments, thanks for all the work in this module!
| description: > | ||
| Statistics that reflect the current use and state of a running `mongod` instance | ||
| for more information, take a look at https://docs.mongodb.com/manual/reference/command/serverStatus/#serverstatus.metrics | ||
| fields: |
There was a problem hiding this comment.
Could you add descriptions to fields too?
| "replset_update_position": c.Dict("replSetUpdatePosition", commandSchema), | ||
| "server_status": c.Dict("serverStatus", commandSchema), | ||
| "update": c.Dict("update", commandSchema), | ||
| "whatsmyuri": c.Dict("whatsmyuri", commandSchema), |
There was a problem hiding this comment.
As we are collecting metrics about each one of the commands here, maybe we want to keep the commands as field names, even if they are camel cased, @ruflin wdyt?
There was a problem hiding this comment.
Tricky one. I would prefer to keep all names lower case if possible but I understand that it might confuse people that search for a specific command. at the same time I would hope autocomplete and when they type the first characters, it becomes clear.
There was a problem hiding this comment.
Ok, let's keep them without camel case by now, we can revisit the decision in the future if it confuses people.
| return nil, err | ||
| } | ||
|
|
||
| data, _ := schema.Apply(result) |
| // New creates a new instance of the MetricSet | ||
| // Part of new is also setting up the configuration by processing additional | ||
| // configuration entries if needed. | ||
| func New(base mb.BaseMetricSet) (mb.MetricSet, error) { |
There was a problem hiding this comment.
Add an experimental warning here.
|
jenkins, test this |
|
jenkins, test this |
|
There is an error with |
| - name: attempts_to_become_secondary | ||
| type: long | ||
| - name: batches | ||
| type: groupt |
There was a problem hiding this comment.
@a3dho3yn typo here (it should be group), test is failing because of this.
| - name: ready | ||
| type: int | ||
| - name: free | ||
| type: int |
|
jenkins, test this please |
CHANGELOG.asciidoc
Outdated
| *Metricbeat* | ||
|
|
||
| - Add metrics about cache size to memcached module {pull}7740[7740] | ||
| - Add `locks`, `global_locks`, `oplatencies` and `process` fields to `status` metricset of MongoDB module. {pull}7613[7613] |
There was a problem hiding this comment.
This changelog entry seems incorrect.
| # - dbstats | ||
| # - status | ||
| # - collstats | ||
| # - metrics |
There was a problem hiding this comment.
Could you also add them to the config.reference.yml file?
|
jenkins, test this |
|
jenkins, test this |
Add a metricset to cover the metrics field included in the response of `db.serverStatus()`. (cherry picked from commit 5b7d31c)
The status metricset does not completely reflect db.serverStatus(). So it was requested to add a metric set to cover the
metricsfield.