multiple master with host and port labels #372
Conversation
|
Thanks for the PR! Also, turns out I had Drone (the CI pipeline) for PRs turned off, oops. I turned it back on, could you please give this a little bump to trigger the pipeline and run tests, etc? Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #372 +/- ##
==========================================
+ Coverage 78.64% 78.92% +0.28%
==========================================
Files 2 2
Lines 899 911 +12
==========================================
+ Hits 707 719 +12
Misses 157 157
Partials 35 35
Continue to review full report at Codecov.
|
oliver006
left a comment
oliver006
left a comment
There was a problem hiding this comment.
Two smaller issues/questions but generally looks good, thanks for the PR!
exporter.go
Outdated
| slaveInfoFields = map[string]bool{"master_host": true, "master_port": true, "slave_read_only": true} | ||
| ) | ||
|
|
||
| if strings.HasPrefix(fieldKey, "master_host") { |
There was a problem hiding this comment.
From the snippet you posted in #371 it looks like this will occur in the # Replication section so maybe only pick these up while in the fieldClass == "Replication" section down starting line 810?
There was a problem hiding this comment.
Also, why HasPrefix and not match it exactly?
There was a problem hiding this comment.
I can simplify it.
exporter.go
Outdated
| } | ||
| return true | ||
| } | ||
| if fieldKey == "master_last_io_seconds_ago" || fieldKey == "slave_repl_offset" || fieldKey == "master_sync_in_progress" { |
There was a problem hiding this comment.
Can you redo this if statement with the one from line 714 and make it a switch on fieldKey?
There was a problem hiding this comment.
I can change to switch
|
Thanks for the changes! Two issues or questions:
|
|
Hi, I made some mistakes and ended deleting my branch and making new pull request #374 |
3 participants
Fix for
#371