The Moment of Discovery: How a Test Failure Revealed a Duplicate Metrics Bug in Distributed Storage

Introduction

In the middle of a large-scale test coverage initiative for the Filecoin Gateway's distributed S3 storage system, a single command execution message captures a pivotal moment of discovery. The message, index 2441 in the conversation, is deceptively simple — a bash command running Go unit tests followed by a panic trace. But this message represents far more than a routine test run: it is the precise instant when a latent production bug surfaced, a bug that had been silently waiting to corrupt Prometheus metrics in a live deployment. Understanding why this message was written, what it reveals, and how it shaped the subsequent debugging session offers a window into the rigorous quality practices of distributed systems engineering.

The Message

The assistant executes:

cd /home/theuser/gw && go test ./rbdeal/ --count=1 2>&1

And receives:

--- FAIL: TestGC_Disabled (0.00s)
panic: duplicate metrics collector registration attempted [recovered, repanicked]

goroutine 395 [running]:
testing.tRunner.func1.2({0x13b7600, 0xc000aea100})
	/usr/lib/go/src/testing/testing.go:1872 +0x237
testing.tRunner.func1()
	/usr/lib/go/src/testing/testing.go:1875 +0x35b
panic({0x13b7600?, 0xc000aea100?})
	/usr/lib/go/src/runtime/panic.go:783 +0x132
github.com/prometheus/client_golang/prometheus.(*Registry).MustRegister(0x213e540, {0xc00084d8f0?, 0x14b27c...

The panic originates from Prometheus's MustRegister function, which enforces that each metric collector can only be registered once with the global registry. When a test creates multiple instances of a component that registers metrics, the second registration attempt panics.

Why This Message Was Written: The Broader Context

This message was not written in isolation. It is the culmination of a systematic test coverage initiative that had been running across multiple phases. Earlier in the session, the assistant had identified and fixed a critical production issue: CIDgravity's GBAP (Get Best Available Providers) API was returning NO_PROVIDERS_AVAILABLE, preventing any storage deals from being made. The assistant implemented a configurable fallback provider mechanism (RIBS_DEAL_FALLBACK_PROVIDERS) and deployed it successfully.

With that fire extinguished, the assistant pivoted to a proactive quality initiative: writing comprehensive unit tests across the entire codebase. The user had explicitly requested a testing plan and implementation, using subagents for each area. By the time this message was written, the assistant had already completed:

• Phase 1.1: 14 configuration tests in configuration/config_test.go
• Phase 1.2: 15 CIDgravity client tests in cidgravity/cidgravity_test.go
• Phase 1.3: Fixed previously skipped tests in rbstor/basic_test.go
• Phase 2.1: Deal making tests in rbdeal/group_deal_test.go
• Phase 2.3: 26 deal repair tests in rbdeal/deal_repair_test.go
• Phase 4.2: 42 S3 authentication tests in server/s3/auth_test.go That is over 100 new tests written across multiple subagent tasks. The assistant had just finished running all package tests individually — configuration, cidgravity, and server/s3 all passed cleanly. The next logical step was to verify that the rbdeal package also passed, which is precisely what this message does.

The Reasoning and Motivation

The assistant's motivation for running this specific command is rooted in a disciplined testing workflow. After each subagent completed its test-writing task, the assistant verified that the new tests passed individually. But there is a deeper concern at play here: test pollution. The assistant had already observed a curious behavior in message 2438–2440: when running all four packages together (configuration, cidgravity, rbdeal, server/s3), the rbdeal package failed with TestGC_Disabled, but when running rbdeal alone, it passed. This intermittent failure pattern is a classic symptom of test pollution — where state from one test package leaks into another.

The assistant's reasoning chain is visible in the preceding messages. In message 2438, the assistant runs the combined test suite and spots the failure. In message 2439, the assistant isolates the failing test (TestGC_Disabled) and runs it individually — it passes. In message 2440, the assistant runs each package individually to confirm they all pass in isolation. Message 2441 is the final verification: running just rbdeal alone to confirm the failure is reproducible within the package itself.

The key insight is that the assistant expected rbdeal to pass when run alone, based on the earlier individual run. But it didn't. The panic reveals that the problem is not cross-package pollution but rather intra-package pollution — something within rbdeal itself is causing duplicate metric registration.

Assumptions Made

The assistant operated under several assumptions that this message would challenge:

1. Individual package tests were sufficient isolation. The assistant assumed that running go test ./rbdeal/ would be equivalent to running the package in isolation. While Go's test runner does create a fresh test binary per package, the Prometheus global registry persists across test functions within the same binary unless explicitly reset.
2. The sync.Once fix applied to index_metered.go was sufficient. Earlier in the session, the assistant had fixed a duplicate metrics registration issue in index_metered.go using sync.Once to ensure metrics were registered only once. The assumption was that this was an isolated problem in that file. The gc.go file, which also registers Prometheus metrics, was assumed to be safe.
3. Tests that pass individually indicate correct code. This is a common assumption in testing, but it is false when global state (like Prometheus's default registry) is involved. A test may pass in isolation but fail when run alongside other tests that share the same global registry.
4. The test harness was properly isolated. The assistant had built a YugabyteDB test harness and was using it across multiple tests. The assumption was that each test function would clean up after itself, but the Prometheus metrics registration had no cleanup mechanism.

Mistakes and Incorrect Assumptions

The most significant mistake revealed by this message is the design of the metrics registration pattern itself. The newGCMetrics() function in gc.go used promauto.NewCounter, promauto.NewGauge, and promauto.NewHistogram — convenience wrappers that automatically register metrics with the global Prometheus registry. While convenient for production code where a single instance exists, this pattern is fundamentally incompatible with testing, where multiple instances may be created and destroyed.

The promauto package's MustRegister call panics on duplicate registration rather than returning an error. This is by design in Prometheus — duplicate metric names indicate a programming error in production. However, in test code, creating multiple instances of a component is common practice, and the panic becomes a test-breaking nuisance rather than a helpful guard.

A secondary mistake was the absence of a metrics registry abstraction. The code directly coupled business logic (garbage collection) to infrastructure (Prometheus metrics registration). A better design would inject a registry or use a per-instance metrics namespace, allowing tests to use isolated registries or mock metrics entirely.

Input Knowledge Required

To fully understand this message, a reader needs knowledge of:

• Go testing conventions: How go test works, the --count=1 flag (to disable test caching), and how test binaries are built per package.
• Prometheus client_golang: The promauto package's auto-registration behavior, the MustRegister panic on duplicates, and the concept of a global default registry.
• The sync.Once pattern: A Go concurrency primitive that ensures a function is executed only once, even across multiple goroutines. This is the canonical fix for duplicate registration problems.
• The project architecture: That rbdeal contains both deal-making logic and garbage collection, that GC metrics are defined in gc.go, and that a similar fix was already applied to index_metered.go.
• Test pollution: The phenomenon where state from one test affects another test's behavior, often through global variables, shared databases, or singleton registries.

Output Knowledge Created

This message produced several forms of knowledge:

1. A confirmed bug location: The panic trace directly identifies gc.go as the source of duplicate metrics registration. The goroutine dump and stack trace point to MustRegister being called a second time for the same metric collector.
2. A pattern recognition: The assistant immediately recognized this as "the same issue we fixed earlier for the index_metered.go" (message 2442). This cross-file pattern recognition is valuable architectural knowledge — it suggests that multiple components share the same fragile metrics registration pattern and may all need the same fix.
3. A test coverage gap: The fact that this bug was only discovered when running the full test suite (or the full package) reveals that no existing test exercised the GC component in a way that created multiple instances. The new tests being added (deal repair tests, group deal tests) were the first to trigger this path.
4. A fix strategy: The panic output implicitly prescribes the fix — use sync.Once to guard metrics registration, exactly as was done for index_metered.go. The assistant follows this prescription in messages 2442–2446.

The Thinking Process

The assistant's reasoning is visible across the message sequence surrounding this one. The thought process can be reconstructed as follows:

Step 1 — Observation: The combined test run fails, but individual package runs pass. This suggests test pollution, but the pollution could be cross-package (state leaking between test binaries) or intra-package (state leaking between test functions within the same binary).

Step 2 — Isolation: The assistant isolates the failing test (TestGC_Disabled) and runs it alone — it passes. This rules out a deterministic bug in that specific test function.

Step 3 — Package isolation: The assistant runs each package individually — all pass. This rules out cross-package pollution (since each go test ./pkg/ invocation builds a separate test binary with a fresh Prometheus registry).

Step 4 — Reproduction: The assistant runs go test ./rbdeal/ alone (message 2441) and reproduces the failure. This confirms the bug is within rbdeal itself — some tests within the package are registering metrics that conflict with each other.

Step 5 — Pattern matching: The panic message "duplicate metrics collector registration attempted" immediately triggers recognition. The assistant had fixed this exact pattern in index_metered.go using sync.Once. The assistant now knows exactly where to look and what to do.

Step 6 — Verification and fix: The assistant reads gc.go, confirms the promauto pattern, applies the sync.Once fix, and verifies all tests pass.

Conclusion

Message 2441 is a testament to the value of comprehensive testing. The 100+ new tests being added did not just verify new functionality — they stress-tested existing code paths that had never been exercised in a multi-instance context. The duplicate metrics bug was latent in production code, waiting to cause silent data corruption in Prometheus dashboards if a GC instance were ever restarted or if multiple instances were deployed. The test suite, by creating multiple component instances, exposed this fragility immediately.

More broadly, this message illustrates a fundamental truth about distributed systems testing: global state is the enemy of testability. Prometheus's global registry, while convenient, creates hidden coupling between components that should be independent. The sync.Once fix is pragmatic — it prevents the panic — but the deeper architectural lesson is about designing for testability from the start. A registry injection pattern would have prevented this bug entirely, and the test suite would have been simpler to write.

The message also showcases the assistant's disciplined debugging methodology: observe the failure, isolate the variables, reproduce deterministically, pattern-match against known issues, and apply a proven fix. This is not glamorous work, but it is the foundation of reliable distributed systems.