From ec28a2fdfe015366c3f3303e8700adc5eb4c8744 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 4 Aug 2026 15:49:45 +0700 Subject: [PATCH 1/2] chore(logging): correct wide-event comments, document the sink split MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hot-path wide events (delivery.attempted, event.received) are Info, but several comments around them called them audit events — leftover wording from the multi-line audit calls they replaced. Nothing routes them to the OTel logs sink, so the comments described behavior that doesn't exist. Say where each sink's line belongs once, on logging.Logger, and drop the history and re-explanation from the call sites. Co-Authored-By: Claude Opus 5 (1M context) --- internal/deliverymq/messagehandler.go | 14 +++++--------- internal/logging/logger.go | 6 ++++++ internal/publishmq/eventhandler.go | 5 ++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/deliverymq/messagehandler.go b/internal/deliverymq/messagehandler.go index 30f5943a3..867d86f75 100644 --- a/internal/deliverymq/messagehandler.go +++ b/internal/deliverymq/messagehandler.go @@ -174,9 +174,9 @@ func (h *messageHandler) handleError(msg *mqs.Message, err error) error { } // Attempt errors from a destination's publish call (webhook 5xx, timeout, // refused, etc.) are expected operational outcomes. Ack semantics are - // already decided above; the failure is captured in the audit log, the - // ClickHouse log entry, and the scheduled retry. Suppress propagation so - // the consumer doesn't log them as unexpected handler errors. + // already decided above; the failure is captured in the delivery.attempted + // line, the ClickHouse log entry, and the scheduled retry. Suppress + // propagation so the consumer doesn't log them as unexpected handler errors. if atmErr, ok := err.(*AttemptError); ok { var pubErr *destregistry.ErrDestinationPublishAttempt if errors.As(atmErr.err, &pubErr) { @@ -186,8 +186,8 @@ func (h *messageHandler) handleError(msg *mqs.Message, err error) error { return err } -// retryOutcome captures the retry-scheduling decisions made during a delivery -// attempt so they can be folded into the single delivery.attempted audit event. +// retryOutcome defers the retry-scheduling decisions taken during an attempt +// so logDeliveryResult can fold them into the one delivery.attempted line. type retryOutcome struct { scheduled bool backoff time.Duration @@ -278,10 +278,6 @@ func (h *messageHandler) logDeliveryResult(ctx context.Context, task *models.Del attempt.AttemptNumber = task.Attempt attempt.Manual = task.Manual - // Wide event: one audit per delivery attempt carrying the full outcome - // (attempt result, timing, retry decision). Replaces the separate - // "retry scheduled" and "scheduled retry canceled" audits so consumers - // don't have to join across lines to reconstruct what happened. fields := []zap.Field{ zap.String("attempt_id", attempt.ID), zap.String("event_id", task.Event.ID), diff --git a/internal/logging/logger.go b/internal/logging/logger.go index 6b5defa9a..9e3deeb35 100644 --- a/internal/logging/logger.go +++ b/internal/logging/logger.go @@ -14,6 +14,12 @@ import ( // operator logs (stdout, no OTel export) while auditLogger is otelzap-wrapped // so Audit() lines flow to both stdout and the OTel logs SDK. This keeps // infrastructure errors and debug noise out of customer-visible OTel sinks. +// +// Audit() is for control-plane changes — tenant and destination lifecycle, +// manual retry, auto-disable — one line per user-visible decision. The hot +// path uses Info even for its wide events (delivery.attempted, +// event.received): they scale with throughput, and tenants read delivery +// history through the events and attempts APIs, not this sink. type Logger struct { *zap.Logger auditLogger *otelzap.Logger diff --git a/internal/publishmq/eventhandler.go b/internal/publishmq/eventhandler.go index 9c5fca34f..bedcc1ca2 100644 --- a/internal/publishmq/eventhandler.go +++ b/internal/publishmq/eventhandler.go @@ -78,9 +78,8 @@ func (h *eventHandler) Handle(ctx context.Context, event *models.Event) (*Handle logger := h.logger.Ctx(ctx) receivedAt := time.Now() - // Wide event state: populated by the rest of Handle and emitted as a single - // audit at the end. Replaces the separate "processing event" and per- - // destination "delivery task enqueued" audits. + // Wide event state: accumulated through Handle, emitted as a single + // event.received line by the defer below. var enqueuedMu sync.Mutex var enqueued []string var matched []string From c4a0a1a8eb5e4807dd395e42abfcc3f3eb776fd6 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 4 Aug 2026 16:26:50 +0700 Subject: [PATCH 2/2] chore(logging): move the sink guidance onto Audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The struct doc's job is the type — two sinks, what each is. Which method a call site should pick is doc for Audit, and doesn't need a list of call sites that goes stale. Co-Authored-By: Claude Opus 5 (1M context) --- internal/logging/logger.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/logging/logger.go b/internal/logging/logger.go index 9e3deeb35..960c58315 100644 --- a/internal/logging/logger.go +++ b/internal/logging/logger.go @@ -14,12 +14,6 @@ import ( // operator logs (stdout, no OTel export) while auditLogger is otelzap-wrapped // so Audit() lines flow to both stdout and the OTel logs SDK. This keeps // infrastructure errors and debug noise out of customer-visible OTel sinks. -// -// Audit() is for control-plane changes — tenant and destination lifecycle, -// manual retry, auto-disable — one line per user-visible decision. The hot -// path uses Info even for its wide events (delivery.attempted, -// event.received): they scale with throughput, and tenants read delivery -// history through the events and attempts APIs, not this sink. type Logger struct { *zap.Logger auditLogger *otelzap.Logger @@ -92,6 +86,11 @@ func (l *Logger) Ctx(ctx context.Context) LoggerWithCtx { } } +// Audit records control-plane decisions: lifecycle changes a tenant or +// operator needs a durable record of. +// +// Per-unit-of-work outcomes on the hot path stay on Info however significant +// they are — this sink isn't sized for per-event volume. func (l *Logger) Audit(msg string, fields ...zap.Field) { l.auditLogger.Info(msg, fields...) }