Skip to content

fix: resolve BalanceSyncWorker failing to discover active tenants in multi-tenant mode#1929

Merged
ClaraTersi merged 4 commits intodevelopfrom
fix/balance-sync-worker
Mar 20, 2026
Merged

fix: resolve BalanceSyncWorker failing to discover active tenants in multi-tenant mode#1929
ClaraTersi merged 4 commits intodevelopfrom
fix/balance-sync-worker

Conversation

@ClaraTersi
Copy link
Member

Summary

  • The BalanceSyncWorker and RedisQueueConsumer had "transaction" hardcoded as the service name when calling GetActiveTenantsByService, but tenants are provisioned under the value of APPLICATION_NAME (typically "ledger" in the unified ledger). This caused both workers to receive an empty tenant list and back off indefinitely, logging "no active tenants found, backing off".
  • Adds a serviceName field to both structs, propagated from opts.TenantServiceName through initBalanceSyncWorker and the RedisQueueConsumer init in config.go.
  • Updates all unit, property, and fuzz tests to pass the service name to the constructors.

Root cause

balance.worker.go:192 and redis.consumer.go:134 queried the Tenant Manager with a hardcoded "transaction" service filter, while all other components (PostgreSQL, MongoDB, RabbitMQ) correctly used opts.TenantServiceName. The Tenant Manager returned an empty list because no tenants were associated with the "transaction" service.

Changed files

  • balance.worker.go — new serviceName field, updated constructor and discoverActiveTenants
  • redis.consumer.go — new serviceName field, updated constructor and runMultiTenant
  • config.go — propagates opts.TenantServiceName to both worker constructors
  • workers_multitenant_test.go — updated 5 call sites
  • workers_multitenant_fuzz_test.go — updated 2 call sites
  • workers_multitenant_property_test.go — updated 4 call sites

@ClaraTersi ClaraTersi requested a review from a team as a code owner March 19, 2026 21:02
@coderabbitai
Copy link

coderabbitai bot commented Mar 19, 2026

Walkthrough

Multi-tenant components were made configurable by introducing a serviceName string field on BalanceSyncWorker and RedisQueueConsumer. Their multi-tenant constructors now accept serviceName and use it when calling tenantClient.GetActiveTenantsByService(...) instead of the hardcoded "transaction". Configuration wiring (InitServersWithOptions / initBalanceSyncWorker) now trims and validates opts.TenantServiceName (error if empty when multi-tenant is enabled) and passes the resulting tenantServiceName into multi-tenant constructors. Tests and fuzzers were updated to provide and assert the new serviceName value.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description provides a comprehensive summary covering root cause, solution, and changed files. However, the description template was not followed—only Summary/Root cause/Changed files were provided, with no completion of the checklist items (PR type selection, testing confirmation, documentation updates, security checks, etc.). Complete the PR template checklist: select the appropriate PR type (likely Transaction), verify all checklist items are checked (testing, documentation, coding standards, security, tests passing), and confirm code is ready for review.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main fix: resolving BalanceSyncWorker's failure to discover active tenants in multi-tenant mode, which matches the core problem and solution in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@lerian-studio
Copy link
Contributor

lerian-studio commented Mar 19, 2026

📊 Unit Test Coverage Report: midaz-transaction

Metric Value
Overall Coverage 85.4% ✅ PASS
Threshold 85%

Coverage by Package

Package Coverage
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/grpc/in 100.0%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/http/in 78.5%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/mongodb 66.7%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/assetrate 100.0%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/balance 100.0%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/operation 90.0%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/operationroute 100.0%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/transaction 99.1%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/transactionroute 100.0%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/rabbitmq 93.1%
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/redis/balance 100.0%
github.com/LerianStudio/midaz/v3/components/transaction/internal/services/command 90.4%
github.com/LerianStudio/midaz/v3/components/transaction/internal/services/query 95.2%
github.com/LerianStudio/midaz/v3/components/transaction/internal/services 100.0%

Generated by Go PR Analysis workflow

@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2026

🔒 Security Scan Results — transaction

Filesystem Scan

✅ No vulnerabilities or secrets found.

Docker Image Scan

✅ No vulnerabilities found.

All security checks passed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/transaction/internal/bootstrap/config.go`:
- Line 371: The multi-tenant balance sync workers are created with
opts.TenantServiceName which can be empty/whitespace and cause silent backing
off; validate that opts.TenantServiceName is non-empty (trim whitespace) early
and fail fast (return an error) if invalid, then pass the validated tenant
service name as a parameter into initBalanceSyncWorker (and to
NewBalanceSyncWorkerMultiTenant call sites) instead of having
initBalanceSyncWorker/readers reference opts.TenantServiceName directly; update
initBalanceSyncWorker signature to accept the tenantServiceName string and use
that validated value when constructing balanceSyncWorker and any other
multi-tenant resources.

In `@components/transaction/internal/bootstrap/workers_multitenant_fuzz_test.go`:
- Line 75: The fuzz test currently hardcodes the serviceName argument when
constructing NewBalanceSyncWorkerMultiTenant, so update the fuzz input seeds to
include multiple service-name values (e.g., "", " ", "ledger", "transaction")
and use the fuzzed value for the serviceName parameter in the
NewBalanceSyncWorkerMultiTenant calls (both the occurrence that builds worker at
NewBalanceSyncWorkerMultiTenant(...) and the other invocation around line 144)
so the constructor is exercised with varied serviceName inputs to catch
misconfiguration regressions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68bfed61-a70e-4079-8eca-edc35f954da0

📥 Commits

Reviewing files that changed from the base of the PR and between 35e4c61 and 7a8462a.

📒 Files selected for processing (6)
  • components/transaction/internal/bootstrap/balance.worker.go
  • components/transaction/internal/bootstrap/config.go
  • components/transaction/internal/bootstrap/redis.consumer.go
  • components/transaction/internal/bootstrap/workers_multitenant_fuzz_test.go
  • components/transaction/internal/bootstrap/workers_multitenant_property_test.go
  • components/transaction/internal/bootstrap/workers_multitenant_test.go

@ClaraTersi ClaraTersi requested a review from a team as a code owner March 19, 2026 21:21
@lerian-studio
Copy link
Contributor

📊 Unit Test Coverage Report: midaz-ledger

Metric Value
Overall Coverage 87.1% ✅ PASS
Threshold 85%

Coverage by Package

Package Coverage
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/http/in 86.8%

Generated by Go PR Analysis workflow

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/ledger/Dockerfile`:
- Line 2: Remove the unrelated comment "# ci: force rebuild" from the Dockerfile
change in this PR (or revert that single-line change), and if the rebuild
trigger was intentional, move that change into a separate PR or commit with an
explanatory note so this PR remains focused on the multi-tenant service name
discovery fixes; target the commit that introduced the "# ci: force rebuild"
line for removal or relocation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 95763cb2-0f22-46fb-a855-259c340c1909

📥 Commits

Reviewing files that changed from the base of the PR and between 49c0fde and 10059d8.

📒 Files selected for processing (1)
  • components/ledger/Dockerfile

@github-actions
Copy link
Contributor

🔒 Security Scan Results — ledger

Filesystem Scan

✅ No vulnerabilities or secrets found.

Docker Image Scan

✅ No vulnerabilities found.

All security checks passed.

Copy link

@gandalf-at-lerian gandalf-at-lerian left a comment

Choose a reason for hiding this comment

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

The fix is correct and well-scoped.

Root cause identified cleanly: hardcoded "transaction" in both discoverActiveTenants and runMultiTenant while the unified ledger registers under APPLICATION_NAME ("ledger") — so the Tenant Manager always returned an empty list and both workers backed off indefinitely.

What I checked:

  • config.go: early strings.TrimSpace + fail-fast error when MultiTenantEnabled && TenantServiceName == "" is the right pattern. Much better than discovering the misconfiguration only at runtime via backoff loops.
  • balance.worker.go / redis.consumer.go: minimal, surgical change — new serviceName field, constructor propagation, single call site updated. No unrelated diff.
  • Tests: unit, property, and fuzz all updated. The fuzz test still hardcodes "transaction" as the seed value (CodeRabbit flagged this initially), but given that the validation now happens in config.go before workers are ever constructed, the fuzz coverage gap is acceptable. Not blocking.
  • Dockerfile: the comment cleanup (# ci: force rebuild to sync ledger image with beta.10# ci: force rebuild) is harmless noise but mildly distracting in a focused fix PR. No impact.
  • CI: coverage 85.4% (transaction) / 87.1% (ledger), both above threshold. Security scans clean.

✅ Approve.

Copy link

@gandalf-at-lerian gandalf-at-lerian left a comment

Choose a reason for hiding this comment

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

The fix is correct and well-scoped.

Root cause identified cleanly: hardcoded "transaction" in both discoverActiveTenants and runMultiTenant while the unified ledger registers under APPLICATION_NAME ("ledger") — so the Tenant Manager always returned an empty list and both workers backed off indefinitely.

What I checked:

  • config.go: early strings.TrimSpace + fail-fast error when MultiTenantEnabled && TenantServiceName == "" is the right pattern. Much better than discovering the misconfiguration only at runtime via backoff loops.
  • balance.worker.go / redis.consumer.go: minimal, surgical change — new serviceName field, constructor propagation, single call site updated. No unrelated diff.
  • Tests: unit, property, and fuzz all updated. The fuzz test still hardcodes "transaction" as the seed value (CodeRabbit flagged this initially), but given that the validation now happens in config.go before workers are ever constructed, the fuzz coverage gap is acceptable. Not blocking.
  • Dockerfile: the comment cleanup is harmless noise but mildly distracting in a focused fix PR. No impact.
  • CI: coverage 85.4% (transaction) / 87.1% (ledger), both above threshold. Security scans clean.

✅ Approve.

@ClaraTersi ClaraTersi merged commit 87abc68 into develop Mar 20, 2026
31 checks passed
@ClaraTersi ClaraTersi deleted the fix/balance-sync-worker branch March 20, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants