Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Structured error contract and validation handling pkg/models/structured_error.go, pkg/models/validationissue.go, openmeter/billing/validationissue.go, pkg/framework/commonhttp/errors_test.go, test/billing/lineengine_test.go |
System errors and validation issues share an implementation but remain distinct. Billing conversion requires explicit conversion. Tests cover structured errors in billing callbacks and HTTP encoding. |
Error-chain diagnostic logging pkg/errorsx/*, openmeter/ledger/README.md |
The shared error handler adds deduplicated codes and reasons from error chains to logs. Traversal is bounded, and the README describes the diagnostic conventions. |
Structured reasons for ledger failures openmeter/ledger/errors.go, openmeter/ledger/advance/*, openmeter/ledger/breakage/*, openmeter/ledger/collector/correction/*, openmeter/ledger/creditvoid/*, openmeter/ledger/fbo/consume.go, openmeter/ledger/recognizer/*, openmeter/ledger/transactions/* |
Ledger failure paths now return structured errors with stable reason attributes. Tests inspect selected reasons and failed transaction state. |
Negative FBO balance reporting and meter wiring openmeter/ledger/fbo/*, openmeter/ledger/collector/service.go, app/common/*, openmeter/billing/charges/service/*, openmeter/billing/worker/subscriptionsync/service/*, openmeter/ledger/chargeadapter/*, openmeter/ledger/collector/collection_fbo_test.go, openmeter/ledger/customerbalance/testenv_test.go, test/* |
FBO and collector configurations require meters. Negative source balances increment an observation counter and produce warning logs while remaining excluded from spendable balances. Test setups provide no-op meters. |
Priority:
Estimated code review effort: 4 (Complex) | ~60 minutes
Change: Feature
Suggested labels: area/billing
Suggested reviewers: turip
Merge Risk: 
29034
-
The documentation change is consistent with the implemented error distinctions and is mergeable.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely summarizes the main changes: ledger accounting diagnostics and negative-balance telemetry. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
3f553b1 to
dcbd79d
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
dcbd79d to
301b485
Compare
| func (s *service) recognitionGroup(ctx context.Context, namespace, id string, groups map[string]ledger.TransactionGroup) (ledger.TransactionGroup, error) { | ||
| if id == "" { | ||
| return nil, fmt.Errorf("recognition lineage is missing its allocation or backing transaction group") | ||
| return nil, models.NewError("ledger_recognition_failed", "recognition lineage is missing its allocation or backing transaction group", models.WithAttribute("reason", "ledger_recognition_lineage_reference_missing")) |
There was a problem hiding this comment.
Please use a constant for the error codes.
| }, | ||
| }, nil | ||
| case models.Error: | ||
| if ordinaryErrorSeverity == "" { |
There was a problem hiding this comment.
Please add a comment here, that any generic error not wrapped using WrapAsValidationIssue is considered a system error.
bea52da to
299ba20
Compare
299ba20 to
29034a0
Compare
Negative FBO source minima emit a counter and diagnostic warning on every observation, including repeated reads. Logs include the source, query boundary, and minimum balance; the counter measures observations, not distinct failures.
Ledger accounting guards gain stable codes and reasons, surfaced by the shared error logger through wrapped and joined errors. Diagnostic traversal is bounded.
Introduce
models.Errorwith the existing structured-error capabilities while keepingmodels.ValidationIssuea distinct type. Both share the implementation and preserve cause traversal and identity through context enrichment. Billing propagates structured system failures instead of storing them as validation issues, even when their cause is a validation issue.Validation on the rebased stack: model, logging, HTTP, billing classification, PostgreSQL-backed ledger, and invoice error-propagation tests plus scoped Go lint passed.
Stack: #5252 → #5270 → #5294 → #5304. Depends on #5294.
Summary by CodeRabbit
New Features
Improvements
Documentation
The PR appears safe to merge; no actionable new issue remains.Summary
Adds accounting error codes and reasons, shared diagnostic logging, and a counter and warning for each negative FBO source observation.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD E[Wrapped or joined error] --> C{Error kind} C -->|models.Error| F[Return operation failure] C -->|models.ValidationIssue| V[Return validation issues] C -->|Explicit billing conversion| V E --> L[Shared error logger] L --> D[Collect current codes and reasons]Reviews (6) · Last reviewed commit: "refactor(ledger): name diagnostic error ..." · Reviewed by Greptile