feat(audit): session ID generation, sequence counter, and BoundaryLog wiring#196
Open
feat(audit): session ID generation, sequence counter, and BoundaryLog wiring#196
Conversation
Add SessionID field to config.AppConfig and generate a UUIDv4 in Run (run_linux.go) at process startup. The session ID is logged once and will be used by future work to group all audit events from a single boundary invocation into a session. No consumers yet; this is the first step of the session correlation RFC.
Introduce SequenceCounter, a shared atomic uint64 counter intended to be initialized once per boundary session and shared between the socket auditor and the proxy. The Next() method returns a zero-based, monotonically increasing value so the audit event and any injected header for the same request agree on the sequence number. No wiring yet; this commit only adds the type and its tests.
… BoundaryLogs Wire the session ID (from PR 1) and SequenceCounter (from PR 2) into SocketAuditor so that every emitted BoundaryLog carries a sequence number and every flushed ReportBoundaryLogsRequest carries the session ID. Changes: - socket_auditor.go: Add sessionID and seq fields to SocketAuditor. AuditRequest now sets SequenceNumber via seq.Next(). flush() accepts and sets SessionId on ReportBoundaryLogsRequest. - multi_auditor.go: SetupAuditor accepts sessionID, creates a SequenceCounter, and passes both to NewSocketAuditor. - landjail/parent.go, nsjail_manager/parent.go: Pass config.SessionID to SetupAuditor. - go.mod: Bump coder/coder to the commit that adds SequenceNumber and SessionId to the generated proto types. - socket_auditor_test.go: Update test helpers to set the new fields. Add TestSocketAuditor_AuditRequest_SequenceNumberIncrements and TestSocketAuditor_Loop_FlushIncludesSessionID.
This was referenced Apr 30, 2026
SasSwart
changed the title
SasSwart
changed the base branch from
sasswart/feat-audit-sequence-counter
to
main
SasSwart
commented
May 1, 2026
Contributor
Author
SasSwart
left a comment
SasSwart
left a comment
There was a problem hiding this comment.
Self review complete. A few notes and questions. Nothing major.
| // and conditionally adds a SocketAuditor if audit logs are enabled and the | ||
| // workspace agent's log proxy socket exists. | ||
| func SetupAuditor(ctx context.Context, logger *slog.Logger, disableAuditLogs bool, logProxySocketPath string) (Auditor, error) { | ||
| func SetupAuditor(ctx context.Context, logger *slog.Logger, disableAuditLogs bool, logProxySocketPath string, sessionID string) (Auditor, error) { |
| agentWillProxy := !os.IsNotExist(err) | ||
| if agentWillProxy { | ||
| socketAuditor := NewSocketAuditor(logger, logProxySocketPath) | ||
| seq := &SequenceCounter{} |
Contributor
Author
There was a problem hiding this comment.
This isn't currently included in the log auditor. Only the socket auditor. Does that matter? I don't think so, but I'm open to the idea.
| ctx := context.Background() | ||
|
|
||
| auditor, err := SetupAuditor(ctx, logger, true, "") | ||
| auditor, err := SetupAuditor(ctx, logger, true, "", "test-session") |
Contributor
Author
There was a problem hiding this comment.
Do we need any new test cases for the multi auditor? I think higher level integration tests are a good point to start at for now. Those will be written on top of the PR stack once all of the moving parts are in place.
Member
There was a problem hiding this comment.
Agreed regarding integration tests. Separate PR on top of the stack is fine.
| batchSize int | ||
| batchTimerDuration time.Duration | ||
| socketPath string | ||
| sessionID string |
Contributor
Author
There was a problem hiding this comment.
again, session id should be UUID
johnstcn
reviewed
May 1, 2026
Member
There was a problem hiding this comment.
Nit: This is basically testing atomic.Int32. The only real valid test I could see is testing that it starts at zero and increments.
johnstcn
reviewed
May 1, 2026
Comment on lines
+36
to
+38
| if log.Time == nil { | ||
| t.Fatal("expected Time to be set") | ||
| } |
Member
There was a problem hiding this comment.
also validate that it's not the zero time
johnstcn
approved these changes
May 1, 2026
Member
johnstcn
left a comment
johnstcn
left a comment
There was a problem hiding this comment.
No blocking comments apart from what has already been noted.
- Change sessionID type from string to uuid.UUID across SetupAuditor, NewSocketAuditor, SocketAuditor struct, flush(), config.AppConfig, and callers. Call .String() only at the proto serialization boundary in flush(). - Remove TestSequenceCounter_ConcurrentAccess and TestSequenceCounter_IndependentInstances since they only test atomic.Int32 stdlib behavior. - Add zero-time assertion in TestSocketAuditor_AuditRequest_QueuesLog to validate Time is not the zero time after checking for nil.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Map
RFC: Bridge ↔ Boundaries Correlation
Generate a per-session UUID at startup, introduce an atomic sequence counter, and wire both into
SocketAuditorso that every emittedBoundaryLogcarries asequence_numberand every flushedReportBoundaryLogsRequestcarries thesession_id.Depends on coder/coder#24809 (proto bump).
Note
This PR was authored by Coder Agents.