-
Notifications
You must be signed in to change notification settings - Fork 453
Restore server status collector #557
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
Conversation
| logger.Debug("serverStatus result:") | ||
| debugResult(logger, m) | ||
|
|
||
| for _, metric := range makeMetrics("", bson.M{"serverStatus": m}, d.topologyInfo.baseLabels(), d.compatibleMode) { |
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.
The nesting here facilitates the desired renaming and label management in makeMetrics in the same manner as when fetched via getDiagnosticData.
| debugResult(logger, m) | ||
|
|
||
| for _, metric := range makeMetrics("", m, d.topologyInfo.baseLabels(), d.compatibleMode) { | ||
| for _, metric := range makeMetrics("", bson.M{"replSetGetStatus": m}, d.topologyInfo.baseLabels(), d.compatibleMode) { |
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.
The nesting here facilitates the desired renaming and label management in makeMetrics in the same manner as when fetched via getDiagnosticData.
This is a breaking change when running with --collector.replicasetstatus as these metrics were previously prefixed with mongodb_ and will now be prefixed with mongodb_rs_. This achieves parity with the metrics returned when using --collector.diagnosticdata and allows for easy switching between the collection methods.
b0922b9 to
1837b57
Compare
|
@anton-bystrov How would this affect the dashboards? |
I think this is critical, because we have queries with mongodb_. mongodb_rs is another metric. But we can add "or" to prevent this. |
|
@ShashankSinha252 @denisok Please be careful before merging this since it will affect the dashboards |
26df81e to
04f657a
Compare
|
@anton-bystrov @percona-csalguero @ShashankSinha252 @denisok Do you need anything from me to facilitate moving forward with this in order to restore the ability to fetch server status for mongos instances? |
|
@rnovikovP please check |
This reverts commit cf4ca67.
04f657a to
402af79
Compare
|
@denisok @rnovikovP I've rebased to keep this PR up to date. Is this something that could be looked at? |
This is useful when running the exporter against mongos instances where getDiagnosticData is not available.
This change prevents running the replSetGetStatus or serverStatus collectors if those data have been collected already via getDiagnosticData. Provide data from the replSetGetStatus and serverStatus collectors to makeMetrics in a format such that the desired renames and label management actions are carried out. Note that this change is not backwards compatible for anyone running with --collector.replicasetstatus as those metrics will now have a prefix of mongodb_rs_ rather than just mongodb_
402af79 to
40a3201
Compare
|
@BupycHuk please check it out |
|
Hello @trvrnrth, sorry for so long response. |
PMM-7947
This PR allows use of the
serverStatusandreplSetGetStatuscollectors for scenarios where fetching all the data provided bygetDiagnosticDatais either not desirable (or in the case of mongos not possible).Metric naming conventions and labels will now be consistent regardless of the collector in use (eg. metric prefixes will always be
mongodb_ss_andmongodb_rs_).Note that this is a breaking change for users running with
--collector.replicasetstatusasmongodb_members_uptimewill now bemongodb_rs_members_uptimefor example.Once all checks pass and the code is ready for review, please add
pmm-review-exportersteam as the reviewer. That would assign people from the review team automatically. Report any issues on our Forum or Discord.