Skip to content

Fix inverted mongodb_mongos_sharding_chunks_is_balancer_running gauge #1295

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

Open
nisarg14 wants to merge 7 commits into
base: main
Choose a base branch
from
+71 −5
Open

Fix inverted mongodb_mongos_sharding_chunks_is_balancer_running gauge #1295

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ed07046
Fix invertedmongodb_mongos_sharding_chunks_is_balancer_running gauge
nisarg14 May 28, 2026
7a9538c
Merge branch 'main' into fix/in-balancer-round-metric
ademidoff Jun 1, 2026
9c0c952
Merge branch 'main' into fix/in-balancer-round-metric
ademidoff Jun 1, 2026
933863b
extracted bool->value mapping in pure function and added unit test fo…
nisarg14 Jun 2, 2026
fd2a010
Fix field reference casing in chunksBalancerRunning
nisarg14 Jun 6, 2026
9d53c82
Update exporter/v1_compatibility_test.go
ademidoff Jun 16, 2026
16320bf
format source code for Fix inverted mongodb_mongos_sharding_chunks_is…
nisarg14 Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion exporter/v1_compatibility.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ func chunksBalancerRunning(ctx context.Context, client *mongo.Client) (prometheu
}

value := float64(0)
if !m.InBalancerRound {
if m.InBalancerRound {
value = 1
}

Expand Down
35 changes: 34 additions & 1 deletion exporter/v1_compatibility_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func TestCreateOldMetricFromNew(t *testing.T) {

func TestMongosMetrics(t *testing.T) {
t.Parallel()
t.Run("test mongodb_mongos_sharding_chunks_is_balancer_running metric", func(t *testing.T) {
t.Run("test mongodb_mongos_sharding_balancer_enabled metric", func(t *testing.T) {
type bss struct {
Mode string `bson:"mode"`
}
Expand Down Expand Up @@ -245,6 +245,39 @@ func TestMongosMetrics(t *testing.T) {
}
assert.Equal(t, float64(expected), m.GetGauge().GetValue()) //nolint
})

t.Run("test mongodb_mongos_sharding_chunks_is_balancer_running metric", func(t *testing.T) {
type bss struct {
InBalancerRound bool `bson:"inBalancerRound"`
}

t.Parallel()
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()

port, err := tu.PortForContainer("mongos")
require.NoError(t, err)
client := tu.TestClient(ctx, port, t)

var bs bss
cmd := bson.D{{Key: "balancerStatus", Value: "1"}}
err = client.Database("admin").RunCommand(ctx, cmd).Decode(&bs)
require.NoError(t, err)

var metric prometheus.Metric
var m dto.Metric
metric, err = chunksBalancerRunning(ctx, client)
assert.NoError(t, err)

err = metric.Write(&m)
assert.NoError(t, err)

expected := 0
if bs.InBalancerRound {

@ademidoff ademidoff Jun 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This expected computation mirrors the function under test exactly (chunksBalancerRunning applies the same if inBalancerRound { 1 } mapping), so the assertion is tautological. It does guard against a re-introduced ! inversion, but on an idle test cluster inBalancerRound is effectively always false, so only the → 0 path is ever exercised — the → 1 path (the behavior this PR fixes) is never covered. Consider extracting the bool→value mapping into a pure function and unit-testing both inputs, or at minimum add a comment noting this limitation.

@nisarg14 nisarg14 Jun 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I’ve now extracted the bool->value mapping into a pure function and added a unit test covering both true and false cases. The integration test is kept only to verify the MongoDB response->metric wiring.

@ademidoff ademidoff Jun 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome. One final fix - let's run make format, commit and push - it should make the linter happy :)

@nisarg14 nisarg14 Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure, i've ran make format and pushed the code

@nisarg14 nisarg14 Jul 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Gentle reminder @ademidoff i've ran the command can you please review

expected = 1
}
assert.Equal(t, float64(expected), m.GetGauge().GetValue()) //nolint
})
}

// myState should always return a metric. If there is no connection, the value
Expand Down
Loading