-
Notifications
You must be signed in to change notification settings - Fork 424
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
PMM-13141 Added feature compatibility version collector #863
PMM-13141 Added feature compatibility version collector #863
Conversation
…salguero/mongodb_exporter into PMM-13141_featureCompatibilityVersion
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.
LGTM, just few comments
exporter/exporter.go
Outdated
@@ -70,6 +70,9 @@ type Opts struct { | |||
EnableCollStats bool | |||
EnableProfile bool | |||
EnableShards bool | |||
EnableFCV bool // Feature Compatibility Version. | |||
|
|||
FCVInterval time.Duration |
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'm not sure if we need separate interval for FCV.
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 value changes only after upgrades so it should have a big interval to avoid unnecessary reads
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.
that's true, but it's not a good practice in exporter development. Is this operation generate a big load on system?
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.
no, not really. We can use one of the default collection intervals
|
||
rawValue := walkTo(m, []string{"featureCompatibilityVersion", "version"}) | ||
if rawValue != nil { | ||
version, err := strconv.ParseFloat(fmt.Sprintf("%v", rawValue), 64) |
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.
wouldn't it better to use hashicorps version library to parse it?
…sion # Conflicts: # go.sum
PMM-13141(https://jira.percona.com/browse/PMM-13141
Added a new collector for Feature compatibility version,