Skip to content
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.

Sign up for GitHub

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

Jump to bottom

new metrics #576

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

new metrics #576

wants to merge 3 commits into from

Conversation

dbernstein1
Copy link

PMM-XXXX (optional, if ticket reported)

  • Links to other linked pull requests (optional).
  • Tests passed.
  • Fix conflicts with target branch.
  • Update jira ticket description if needed.
  • Attach screenshots/console output to confirm new behavior to jira ticket, if applicable.

Once all checks pass and the code is ready for review, please add pmm-review-exporters team as the reviewer. That would assign people from the review team automatically. Report any issues on our Forum or Discord.

@dbernstein1 dbernstein1 requested a review from a team as a code owner October 11, 2022 00:10
@dbernstein1 dbernstein1 requested review from ShashankSinha252 and tshcherban and removed request for a team October 11, 2022 00:10
@it-percona-cla
Copy link

it-percona-cla commented Oct 11, 2022 •
edited

CLA assistant check
All committers have signed the CLA.

@BupycHuk
Copy link
Member

Hi, @dbernstein1 thanks for the contribution. Could you provide information on what this PR is about and what problem you are fixing here? Thanks

BupycHuk
BupycHuk reviewed Oct 11, 2022
View reviewed changes
exporter/v1_compatibility.go Outdated
func chunksTotal(ctx context.Context, client *mongo.Client) (prometheus.Metric, error) {
n, err := client.Database("config").Collection("chunks").CountDocuments(ctx, bson.M{})
if err != nil {
@ -1200,9 +1317,26 @@ func chunksTotal(ctx context.Context, client *mongo.Client) (prometheus.Metric,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line looks pretty weird

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry fixed it

Also I tried running make format but got these issues:

go mod tidy
go mod tidy: go.mod file indicates go 1.17, but maximum supported version is 1.16
make: *** [format] Error 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What version of golang do you have locally?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.16.5

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update to at least 1.17.

@dbernstein1 dbernstein1 force-pushed the new-metrics branch 2 times, most recently from 4f741a9 to 97660e6 Compare October 11, 2022 18:03
@dbernstein1
Copy link
Author

@BupycHuk

  1. added the following metrics:
  • collections with chunks currently being balanced among shards (mongodb_mongos_sharding_migrating)
  • System balancer running on/off indictation (mongodb_mongos_sharding_balancer_running)
  • chunk size in megabytes (mongodb_mongos_sharding_chunk_size_mb)

  1. Included a label with the db/collection in mongodb_mongos_sharding_shard_chunks_total to see how chunks are balanced across shards in a given collection

  2. Fixed a bug in balancerEnabled mongodb_mongos_sharding_balancer_enabled always returned 1 even when balanced was disabled

@dbernstein1 dbernstein1 changed the title Update v1_compatibility.go new metrics Oct 12, 2022
@dbernstein1 dbernstein1 force-pushed the new-metrics branch 2 times, most recently from b7c7d00 to 3fbd7b9 Compare October 17, 2022 18:26
BupycHuk
BupycHuk reviewed Oct 21, 2022
View reviewed changes
exporter/v1_compatibility.go
Comment on lines +1243 to +1245
metric, err := prometheus.NewConstMetric(d, prometheus.GaugeValue, float64(running))

return metric, err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
metric, err := prometheus.NewConstMetric(d, prometheus.GaugeValue, float64(running))
return metric, err
return prometheus.NewConstMetric(d, prometheus.GaugeValue, float64(running))

exporter/v1_compatibility.go Outdated
func chunksTotal(ctx context.Context, client *mongo.Client) (prometheus.Metric, error) {
n, err := client.Database("config").Collection("chunks").CountDocuments(ctx, bson.M{})
if err != nil {
@ -1200,9 +1317,26 @@ func chunksTotal(ctx context.Context, client *mongo.Client) (prometheus.Metric,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update to at least 1.17.

@BupycHuk
Copy link
Member

@dbernstein1 please sign the CLA

@dbernstein1
Copy link
Author

@BupycHuk I already did but it is showing up that I didn’t
Screen Shot 2022-10-21 at 3 53 01 PM

@dbernstein1
Copy link
Author

Screen Shot 2022-10-21 at 3 55 25 PM

@dbernstein1 dbernstein1 requested review from BupycHuk and removed request for ShashankSinha252 and tshcherban November 3, 2022 17:23
@artemgavrilov
Copy link
Contributor

Hi @dbernstein1 your commits signed with dabernst@cisco.com email address, but your GH account has different address. Because of that CLA check is not passing.

@BupycHuk
Copy link
Member

BupycHuk commented Dec 5, 2022

@dbernstein1 please add the email address used for this commit to your account

@dbernstein1
Copy link
Author

@BupycHuk I added dabernst@cisco.com to my github account. Tried signing the CLA again

Screenshot 2022-12-05 at 6 36 06 PM

@artemgavrilov
Copy link
Contributor

@dbernstein1 @BupycHuk CLA problem has gone.

BupycHuk
BupycHuk requested changes Dec 7, 2022
View reviewed changes
Copy link
Member

@BupycHuk BupycHuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dbernstein1, Thank you for your contribution and fixing a problem with CLA.

v1_compatibilty collector is supposed to be a collector to keep compatibility with mongodb_exporter v1. For new metrics it's better to create a new collector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BupycHuk BupycHuk BupycHuk requested changes

@ShashankSinha252 ShashankSinha252 Awaiting requested review from ShashankSinha252

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@dbernstein1 @it-percona-cla @BupycHuk @artemgavrilov @denisok @ShashankSinha252