Skip to content

Hardening: expose audit producer failures and document semantics - #3

Merged
SShogun merged 1 commit into
masterfrom
hardening/audit-producer-semantics
Sep 15, 2026
Merged

SShogun merged 1 commit into
masterfrom
hardening/audit-producer-semantics

Conversation

@SShogun

@SShogun SShogun commented Sep 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

This is the next two-part RedisForge P0 hardening pass after the Streams recovery and hot-path/Testcontainers PRs.

1. Producer-side audit observability and consistency

  • centralize audit event serialization, timeout, append, metrics, and structured failure logging in internal/audit
  • preserve the existing asynchronous best-effort HTTP contract: item writes do not roll back when audit XADD fails
  • add audit_events_emitted_total{action,status} and audit_emit_latency_ms{action,status} Prometheus metrics
  • surface append failures with event_id, item_id, action, and error fields
  • apply a bounded 5-second append timeout
  • route create and update handlers through the shared emitter
  • close the existing inconsistency where delete operations had a declared deleted audit action but emitted no event
  • add regression coverage proving create/update/delete actions and synchronous append error propagation

2. Explicit failure-semantics documentation

Add docs/failure-semantics.md and align the README/docs with the real system contract:

  • before XADD: producer delivery is best effort; successful item writes can exist without an audit event
  • after successful XADD: consumer recovery is at least once, so duplicate processing is possible
  • malformed events use log + ACK/drop rather than a dead-letter stream
  • cache failures degrade to the fallback repository where applicable
  • Bloom idempotency currently fails open on lookup errors and is not a transactional exactly-once guarantee
  • RediSearch errors fail the request; there is no full-scan fallback

Audit finding

The adjacent write-path audit found that DELETE /v1/items/{id} did not emit an audit event at all, even though the domain model and metrics already listed deleted as a valid audit action. This PR fixes that mismatch without changing HTTP success semantics.

Verification

The final pre-squash tree passed:

gofmt check
go vet ./...
go test -race -count=1 ./...
go test -count=1 ./internal/redisx -run '^$' -bench '^BenchmarkRedisHotPaths$' -benchtime=1x -benchmem
go build ./cmd/redisforge

The branch is squashed to one commit. CI will run again on the exact PR SHA.

Important remaining limitation

This PR makes producer-side failure visible; it does not turn audit emission into a transactional outbox. Producer appends remain asynchronous and best effort, with no durable retry queue or shutdown drain. That limitation is now explicit and measurable rather than silent.

Summary by Sourcery

Harden audit observability and make RedisForge's failure and delivery semantics explicit.

New Features:

  • Add a shared best-effort audit emitter for create, update, and delete item operations with bounded append timeouts, structured failure logging, and Prometheus metrics.
  • Emit audit events for item deletions, aligning delete behavior with the existing audit action model.

Bug Fixes:

  • Expose producer-side audit append failures without changing successful item-write semantics.

Enhancements:

  • Document audit delivery, cache degradation, Bloom idempotency, stream recovery, and search failure semantics.

Documentation:

  • Add and index documentation describing RedisForge's failure and delivery guarantees.

Tests:

  • Add coverage for audit event emission across create, update, and delete operations and synchronous append error propagation.

Centralize best-effort audit emission with structured failure logging and Prometheus producer metrics. Emit create, update, and delete audit events consistently, add regression coverage, and document the exact failure semantics for producer appends, stream recovery, cache degradation, Bloom idempotency, and search.
Copilot AI lite review requested due to automatic review settings September 15, 2026 15:38
@sourcery-ai

sourcery-ai Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

Introduces a shared, observable audit producer for create/update/delete operations—including the previously missing delete event—while retaining non-transactional asynchronous HTTP semantics, and adds documentation and tests that make the resulting failure and delivery guarantees explicit.

Sequence diagram for best-effort audit emission after item writes

sequenceDiagram
    participant Client
    participant Handler
    participant ItemRepo
    participant AuditEmitter
    participant RedisStreams
    participant AuditWorker

    Client->>Handler: HTTP create/update/delete
    Handler->>ItemRepo: Create/Update/Delete
    ItemRepo-->>Handler: Successful item write
    Handler-)AuditEmitter: EmitAsync(created/updated/deleted)
    Handler-->>Client: HTTP success
    AuditEmitter->>AuditEmitter: Emit
    AuditEmitter->>RedisStreams: Append
    alt append succeeds
        RedisStreams-->>AuditEmitter: event_id
        AuditEmitter-->>AuditEmitter: RecordAuditEmit success
        RedisStreams-)AuditWorker: XREADGROUP
        AuditWorker->>RedisStreams: XACK
    else append fails or times out
        RedisStreams-->>AuditEmitter: error
        AuditEmitter-->>AuditEmitter: RecordAuditEmit error
        AuditEmitter-->>AuditEmitter: structured failure log
    end
Loading

State diagram for audit event delivery semantics

stateDiagram-v2
    [*] --> ItemWrite
    ItemWrite --> Scheduled: EmitAsync
    Scheduled --> AppendAttempt: Emit
    AppendAttempt --> LostBeforeRedis: append error or 5s timeout
    AppendAttempt --> Appended: Append succeeds
    Appended --> Processing: XREADGROUP
    Processing --> Acknowledged: processing succeeds and XACK
    Processing --> Pending: processing or ACK fails
    Pending --> Processing: XAUTOCLAIM recovery
    Processing --> Processing: crash before XACK
    LostBeforeRedis --> [*]
    Acknowledged --> [*]
Loading

File-Level Changes

Change Details Files
Centralize producer-side audit emission while preserving asynchronous best-effort HTTP behavior.
  • Add a shared emitter for JSON serialization, a bounded 5-second append context, structured failure logging, and synchronous error propagation.
  • Route create, update, and delete handlers through the emitter without rolling back successful item writes.
  • Restore missing deleted events and add handler coverage for all CRUD audit actions.
internal/audit/emitter.go
internal/audit/emitter_test.go
internal/app/app.go
internal/handlers/items_create.go
internal/handlers/items_update.go
internal/handlers/items_delete.go
internal/handlers/audit_events_test.go
internal/handlers/items_create_test.go
Expose audit producer outcomes through Prometheus metrics.
  • Record append success/error counts by action.
  • Measure serialization-plus-append latency by action and status.
internal/observability/metrics.go
Document the system’s failure and delivery guarantees across project documentation.
  • Define pre-XADD best-effort delivery, post-XADD at-least-once recovery, malformed-event ACK/drop handling, cache fallback behavior, Bloom fail-open semantics, and search failure behavior.
  • Link the new semantics document from the README and documentation index and update architecture, endpoint, feature, and verification descriptions.
  • Clarify that there is no transactional outbox, durable producer retry queue, or shutdown drain.
docs/failure-semantics.md
README.md
docs/README.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the asynchronous emitter or handler wiring is wrong, item writes can succeed without a corresponding audit event, and that audit history cannot be recovered by reverting the change. The impact is limited to bounded gaps in the best-effort audit stream; reverting restores the previous producer implementation but cannot recreate events already missed.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@SShogun
SShogun merged commit 7cdb590 into master Sep 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants