Skip to content

Commit 2e8a14c

Browse files
committed
docs(eventbus,eventlogger,chimux): clarify review feedback; add exporter build-tag guidance
1 parent af8936d commit 2e8a14c

5 files changed

Lines changed: 38 additions & 10 deletions

File tree

modules/chimux/module.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,19 @@ type ChiMuxModule struct {
146146
subject modular.Subject // Added for event observation
147147
// disabledRoutes keeps track of routes that have been disabled at runtime.
148148
// Keyed by HTTP method (uppercase) then the original registered pattern.
149+
// A disabled route short‑circuits matching before reaching the underlying chi mux
150+
// allowing dynamic feature flag style shutdown without removing the route from
151+
// the registry (so it can be re‑enabled later). Patterns are stored exactly as
152+
// originally registered to avoid ambiguity with chi's internal normalized form.
149153
disabledRoutes map[string]map[string]bool
150154
// disabledMu guards access to disabledRoutes for concurrent reads/writes.
151155
disabledMu sync.RWMutex
152156
// routeRegistry tracks registered routes with their methods for runtime management.
153157
routeRegistry []struct{ method, pattern string }
154158
// middleware tracking for runtime enable/disable
155159
middlewareMu sync.RWMutex
156-
middlewares map[string]*controllableMiddleware
157-
middlewareOrder []string
160+
middlewares map[string]*controllableMiddleware // keyed by middleware name provided at registration
161+
middlewareOrder []string // preserves deterministic application order for rebuilds
158162
}
159163

160164
// NewChiMuxModule creates a new instance of the chimux module.

modules/eventbus/memory.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,23 @@ func (m *MemoryEventBus) Publish(ctx context.Context, event Event) error {
214214
return nil
215215
}
216216

217-
// Optional rotation for fairness. We deliberately removed the previous random shuffle fallback
218-
// (when rotation disabled) to preserve deterministic ordering and avoid per-publish RNG cost.
217+
// Optional rotation for fairness.
218+
// Rationale:
219+
// * Deterministic order when rotation disabled (stable slice) improves testability and
220+
// reasoning about delivery ordering.
221+
// * When rotation enabled we perform a logical rotation using an incrementing counter
222+
// rather than allocating + copying on every publish via append/slice tricks or
223+
// performing a random shuffle. This yields O(n) copies only when the starting offset
224+
// changes (and only for length > 1) with no RNG cost and avoids uint64->int casts
225+
// that would require additional lint suppression.
226+
// * Slice re-slicing with append could avoid an allocation in the start!=0 case, but the
227+
// explicit copy keeps the code straightforward and side-effect free (no aliasing that
228+
// could surprise future mutations) while cost is negligible relative to handler work.
229+
// * We intentionally do not randomize: fairness over time is achieved by round‑robin
230+
// style rotation (pubCounter % len) which ensures equal start positions statistically
231+
// without introducing randomness into delivery order for reproducibility.
232+
// If performance profiling later shows this allocation hot, a specialized in-place rotate
233+
// could be introduced guarded by benchmarks.
219234
if m.config.RotateSubscriberOrder && len(allMatchingSubs) > 1 {
220235
pc := atomic.AddUint64(&m.pubCounter, 1) - 1
221236
ln := len(allMatchingSubs) // ln >= 2 here due to enclosing condition

modules/eventbus/metrics_exporters.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@ package eventbus
2121
// go exporter.Run(ctx)
2222
// ... later cancel();
2323
//
24-
// NOTE: Prometheus and Datadog dependencies are optional. If you want to exclude one of these
25-
// exporters for a build, prefer Go build tags (e.g. //go:build !prometheus) with the exporter
26-
// implementation moved to a separate file guarded by that tag, rather than manual comment edits.
27-
// This file keeps both implementations active by default for convenience.
24+
// NOTE: Prometheus and Datadog dependencies are optional. If you want to exclude an exporter
25+
// from a particular build, prefer Go build tags instead of editing this file manually. Example:
26+
// //go:build !prometheus
27+
// // +build !prometheus
28+
// Move the Prometheus collector implementation into a prometheus_collector.go file guarded by
29+
// a complementary build tag (e.g. //go:build prometheus). This keeps the default experience
30+
// simple (both available) while allowing consumers to tailor binaries without forking.
2831

2932
import (
3033
"context"

modules/eventbus/module.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ type EventBusModule struct {
149149
router *EngineRouter
150150
mutex sync.RWMutex
151151
isStarted bool
152-
subject modular.Subject // For event observation (guarded by mutex)
152+
subject modular.Subject // Observer notification target (lazy-created). Guarded by mutex; kept nil until a consumer
153+
// requests observation to avoid allocation for apps that never observe bus events.
153154
}
154155

155156
// DeliveryStats represents basic delivery outcomes for an engine or aggregate.

modules/eventlogger/module.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,12 @@ func (m *EventLoggerModule) OnEvent(ctx context.Context, event cloudevents.Event
603603
queueResult = nil
604604
return
605605
} else {
606-
// Queue is full - drop oldest event and add new one
606+
// Queue is full - drop oldest event and add new one. We log both the incoming event type
607+
// and the dropped oldest event type for observability. This path intentionally avoids
608+
// emitting an operational CloudEvent because the logger itself is not yet started; emitting
609+
// here would risk recursive generation of events that also attempt to enqueue. Once started,
610+
// pressure signals are emitted via BufferFull/EventDropped events on the hot path with
611+
// safeguards to prevent amplification loops (see further below in non-started path logic).
607612
var droppedEventType string
608613
if len(m.eventQueue) > 0 {
609614
// Capture dropped event type for debugging visibility then shift slice

0 commit comments

Comments
 (0)