Skip to content

feat(auditlog): write organization events to audit_log - #2791

Merged
t-kikuc merged 3 commits into
mainfrom
feat/org-audit-log-write-path
Sep 2, 2026
Merged

feat(auditlog): write organization events to audit_log#2791
t-kikuc merged 3 commits into
mainfrom
feat/org-audit-log-write-path

Conversation

@hvn2k1

@hvn2k1 hvn2k1 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Part of #1982, Follows #2789

What this PR does

Adds organization_id to the domain event and stores organization-level (admin) events in the audit_log table — with an empty environment_id, scoped by their organization id — instead of admin_audit_log.

Background / Why this PR is needed

Organization-level events (account changes, environment/project creation, organization updates) go to admin_audit_log, which only system admins can read, so organization admins cannot see their own organization's activity. With the organization_id column in place (#2789), this switches the write path so new organization events land in audit_log where they can be served per-organization. After this PR the only writes to admin_audit_log are a rollout-compatibility fallback for organization-scoped events published by older producers (no organization_id yet), so the history migration that follows can still resolve them; once old producers and queued events are drained, the fallback goes away and the table is frozen. The read path and console filters come in a separate PR.

Points

  • Every NewAdminEvent producer now passes its entity's organization id (organization, project, environment, account, team). Admin subscription events are system-level and pass an empty organization id, so they land in audit_log with both environment_id and organization_id empty — visible to system admins only.
  • Rollout compatibility: an organization-scoped admin event arriving without an organization id (published by an older producer) falls back to admin_audit_log instead of being stranded in audit_log with both scopes empty; a TODO marks the fallback for removal once old messages drain.
  • Event.IsAdminEvent is kept and its notification semantics are untouched (the domain event informer still uses it); only the storage routing changed.
  • The account service's synchronous in-transaction audit writes were switched from admin_audit_log to audit_log. The dedup between that write and the published event is unchanged: same event id, duplicate-key insert → ack.
  • AdminAuditLogStorage is removed from the subscriber persister and wiring, but the storage itself and the read APIs stay until the read-path switch and cleanup.
  • New organization events are not visible in any console until the read-path PR — expected, since nothing reads admin_audit_log in production.

Copilot AI balanced review requested due to automatic review settings August 27, 2026 10:14
Adds organization_id to the domain event and stores organization-level
(admin) events in the audit_log table with an empty environment_id,
scoped by their organization id, instead of admin_audit_log. All
NewAdminEvent producers now pass their entity's organization id;
system-level admin subscription events pass an empty one.

This freezes admin_audit_log: nothing writes to it anymore. The read
path and console filters follow in a separate PR.

Step 2 of #1982.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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.

Pull request overview

Routes organization-level administrative events into the organization-scoped audit_log table.

Changes:

  • Adds organization IDs to domain events and all producers.
  • Persists all events through AuditLogStorage.
  • Updates account transactions, database inserts, wiring, and tests.

Reviewed changes

Copilot reviewed 23 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
proto/proto.lock Records the new event field.
proto/event/domain/event.proto Adds organization_id.
pkg/web/cmd/server/server.go Wires account audit storage.
pkg/team/api/api.go Scopes team events.
pkg/subscription/command/admin_subscription.go Marks subscriptions system-level.
pkg/subscriber/processor/auditlog_persister.go Unifies persistence routing.
pkg/subscriber/processor/auditlog_persister_test.go Tests unified extraction.
pkg/subscriber/cmd/server/server.go Removes admin persister wiring.
pkg/environment/api/project.go Scopes project events.
pkg/environment/api/organization.go Scopes organization events.
pkg/environment/api/environment_v2.go Scopes environment events.
pkg/domainevent/domain/event.go Populates organization IDs.
pkg/auditlog/storage/v2/postgres/sql/auditlog/insert_audit_logs_v2.sql Adds bulk insert column.
pkg/auditlog/storage/v2/postgres/sql/auditlog/insert_audit_log_v2.sql Adds single insert column.
pkg/auditlog/storage/v2/postgres/audit_log.go Supplies PostgreSQL organization values.
pkg/auditlog/storage/v2/mysql/sql/auditlog/insert_audit_logs_v2.sql Adds bulk insert column.
pkg/auditlog/storage/v2/mysql/sql/auditlog/insert_audit_log_v2.sql Adds single insert column.
pkg/auditlog/storage/v2/mysql/audit_log.go Supplies MySQL organization values.
pkg/auditlog/domain/auditlog.go Carries organization scope.
pkg/account/command/account_v2.go Scopes account events.
pkg/account/api/api.go Uses regular audit storage.
pkg/account/api/api_test.go Updates service mock wiring.
pkg/account/api/account.go Writes account audits to audit_log.
pkg/account/api/account_test.go Updates audit-write expectations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread proto/event/domain/event.proto
Address review comments on PR #2791:
- is_admin_event said admin events are stored in the AdminDomainEvent
  and AdminAuditLog tables; they are now stored in the audit_log table
  with an empty environment_id, scoped by organization_id.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hvn2k1
hvn2k1 requested a review from Ubisoft-potato August 28, 2026 04:02
@hvn2k1
hvn2k1 marked this pull request as ready for review August 28, 2026 04:02
@t-kikuc
t-kikuc requested a balanced review from Copilot August 28, 2026 07:24

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.

Pull request overview

Copilot reviewed 23 out of 25 changed files in this pull request and generated 1 comment.

Comment on lines +134 to +136
// Organization-level (admin) events are stored in the audit_log table
// with an empty environment id, scoped by their organization id.
auditlogs = append(auditlogs, domain.NewAuditLog(event, event.EnvironmentId))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread pkg/subscriber/processor/auditlog_persister.go
…ucers

Address review comments on PR #2791:
- An event published by a producer that predates the organization_id
  field unmarshals with an empty organization id. Routing it to
  audit_log would strand it with both scopes empty, unrecoverable by
  the admin_audit_log history migration. Such events now fall back to
  admin_audit_log until old producers and queued events are drained.
- System-level entities (admin account, admin subscription) legitimately
  have no organization and still go to audit_log.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@t-kikuc t-kikuc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: The bullet "AdminAuditLogStorage is removed from the subscriber persister and wiring" is stale — the rollout fallback (0bdd0ae) keeps it. Consider removing that bullet.

}
// The legacy organization-scoped event goes to admin_audit_log so the
// history migration can resolve its organization later.
assert.Len(t, adminAuditLogs, 1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

assert.Len does not stop the test, so if a routing regression sends the legacy event to auditlogs instead, adminAuditLogs[0].Id on the next line panics with an index-out-of-range and the package's other tests never report. Use require.Len here.

Suggested change
assert.Len(t, adminAuditLogs, 1)
require.Len(t, adminAuditLogs, 1)

@t-kikuc
t-kikuc merged commit b3df88e into main Sep 2, 2026
13 checks passed
@t-kikuc
t-kikuc deleted the feat/org-audit-log-write-path branch September 2, 2026 03:02
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.

4 participants