mongodb_exporter icon indicating copy to clipboard operation
mongodb_exporter copied to clipboard

replSetGetConfigCollector

Open hiroshi opened this issue 3 years ago • 14 comments

Add replSetGetConfigCollector. This is meant to resolve https://github.com/percona/mongodb_exporter/issues/292

It export metrics like this:

mongodb_cfg_members_priority{cl_id="000000000000000000000000",cl_role="shardsvr",member_idx="db-1:27017",rs_nm="MyRep",rs_state="2"} 3
mongodb_cfg_members_hidden{cl_id="000000000000000000000000",cl_role="shardsvr",member_idx="db-1:27017",rs_nm="MyRep",rs_state="2"} 0

Usage example

Count number of members eligible for primary count(mongodb_cfg_members_priority > 0 and mongodb_cfg_members_hidden == 0 and ignoring(member_state) mongodb_members_health == 1) (I use this for alerting when the number is below 2)

TODO

  • [ ] Fix lint errors
  • [x] Add replset_config_collector_test.go

  • [x] Tests passed.
  • [x] Fix conflicts with target branch.
  • [ ] ~~Update jira ticket description if needed.~~
  • [ ] ~~Attach screenshots/console output to confirm new behavior to jira ticket, if applicable.~~

When all checks have passed and code is ready for the review, bot should add pmm-review-exporters team as the reviewer. That would assign people from the review team automatically. Report any issues on our Forums and Discord.

hiroshi avatar Jun 12 '21 02:06 hiroshi

Hi. Just wanted to let you know I haven't forget about this. I've just been busy and I would like to manually test it before merging it. Is it ready for review? It appears as draft.

Regards

percona-csalguero avatar Jul 31 '21 13:07 percona-csalguero

Hi, thanks for your attention.

I think at least a test is needed to be reviewable. So it remains as a draft.

I'll add a test when I have a time. Or, if you think this changes are usable I appreciate you take over this.

hiroshi avatar Aug 02 '21 01:08 hiroshi

Hello again. I've checked out your branch and it looks good. I need to ask you for some small changes:

  • Address the linter suggestions
  • Add some tests, using this collector against the different server types we have in the docker-compose file.

Thanks for your help.

percona-csalguero avatar Aug 04 '21 13:08 percona-csalguero

@hiroshi Do you planning write some tests here? I think everything else is fine and can be merged then.

JiriCtvrtka avatar Sep 16 '21 07:09 JiriCtvrtka

EDIT: I gave up running linter locally. Tried to fix the linter error and checked it by pushing commits.. So you can ignore this comment.


https://github.com/percona/mongodb_exporter/pull/295#pullrequestreview-714630726

Could you address the linter suggestions to set a base for further improvements?

I was trying to run linter on my local Apple Silicon mac, but not succeeded yet. Image from Gyazo

I think this issue is related https://github.com/percona/mongodb_exporter/issues/286

How do I run those linter locally to check if my change is OK?

hiroshi avatar Sep 19 '21 11:09 hiroshi

The line where the linter reported an error, https://github.com/percona/mongodb_exporter/pull/295/files#diff-36945caa3d7bc040bc4a27f3cd597e18399e05ec94a87f344ac74eac5efeb5bcR53 https://github.com/percona/mongodb_exporter/blob/be177633274f738b84a2d0baee24f68acad83616/exporter/replset_config_collector.go#L53 is a almost identiacal to the line, https://github.com/percona/mongodb_exporter/blob/be177633274f738b84a2d0baee24f68acad83616/exporter/replset_status_collector.go#L52 but it seems no linter error here.

I'm not sure what difference cause the linter error...

hiroshi avatar Sep 19 '21 12:09 hiroshi

Also I was trying to run test on my local Apple silicon mac, but unable to run them.

Image from Gyazo

I created another pull request to address it -> https://github.com/percona/mongodb_exporter/pull/342

hiroshi avatar Sep 19 '21 12:09 hiroshi

I fear that to run lint and test on my Apple silicon mac is unneccesary harder than fix lint and add test itself... I hope someone taking over this pull request, fix lint and add test for it. Or should I try on a linux environment, utilize docker for lint and test or fix all problem on Apple silicon mac? Sorry, I'v just been frustrated...

hiroshi avatar Sep 19 '21 12:09 hiroshi

https://github.com/percona/mongodb_exporter/pull/295#pullrequestreview-714630726

Could you address the linter suggestions to set a base for further improvements?

@percona-csalguero I fixed the linter error in 54ded8c.

EDIT: No, this causes segfault on d.logger.Errorf("cannot get replSetGetConfig: %s", err) https://github.com/percona/mongodb_exporter/pull/295/checks

https://github.com/percona/mongodb_exporter/blob/22b9fe372e0bac1c66506516728d7ff096293893/exporter/replset_config_collector.go#L53-L59

hiroshi avatar Sep 20 '21 06:09 hiroshi

Well, I tried to fix the lint error 54ded8c, then add tests for replSetGetConfigCollector 22b9fe3, but it causes segfault https://github.com/percona/mongodb_exporter/pull/295/checks.

Reverting 54ded8c fixs the segfault.

I'm wondering how to fix the lint error. Can you help this?

hiroshi avatar Sep 20 '21 10:09 hiroshi

Just add the //nolint in that line. We won't get any benefit of making it more complex to satisfy the linter.

percona-csalguero avatar Sep 20 '21 12:09 percona-csalguero

I'v just suppressed a set of lint errors at bb9ecc8. However I found that more lint errors in replset_config_collector_test.go...

OK, I'll try to mitigate those error if possible....

I just copied implementation of replset_config_collector_test.go from replset_status_collector_test.go (also replset_config_collector.go). Why no lint errors in replset_status_collector_test.go? Are there any exclusion setting? or lint process only changed file?

EDIT: Also I wonder why the check doesn't failed by those lint errors?

hiroshi avatar Sep 21 '21 01:09 hiroshi

Hello @hiroshi Could you fix the merge conflicts and, if it is ready, mark it as ready for review. Ignore linter errors if that stopping you. Regards

percona-csalguero avatar Feb 22 '23 13:02 percona-csalguero

I'v just tried to resolve those conflict, but there are too much differences since the last merge in origin/main. https://github.com/percona/mongodb_exporter/compare/4f3568b..b526414

It seems that I need to read new code base to adjust my changes....

hiroshi avatar Feb 24 '23 08:02 hiroshi