John/audit ef 9 - #5892
Draft
johnsimons wants to merge 21 commits into
Draft
John/audit ef 9#5892johnsimons wants to merge 21 commits into
johnsimons wants to merge 21 commits into
Conversation
johnsimons
force-pushed
the
john/audit_ef_9
branch
from
September 15, 2026 04:29
409a5ed to
63aee13
Compare
This design document outlines the architectural strategy and work plan for moving audit ingestion into the primary ServiceControl process for SQL Server and PostgreSQL persistence, including support for scaled-out ingestion-only workers.
First step of hosting audit ingestion in the primary instance. Contracts only, no runtime behavior change: nothing resolves or calls any of this yet, and no shipped persister advertises audit support. - IIngestionUnitOfWork gains a nullable Audit child alongside Monitoring and Recoverability, so a batch can record audit messages and saga snapshots in the same transaction as known endpoints. - New query contracts for the two entry points that are remote-only today: IAuditCountsDataStore and ISagaHistoryDataStore. - IFailedAuditImportDataStore mirrors IFailedErrorImportDataStore, and FailedAuditImport.DeriveKey gives failed audit imports the same deterministic key with native-id fallback, so competing consumers do not write a row per delivery attempt. - PersistenceManifest gains SupportsAuditIngestion. Absent means false, so RavenDB and every legacy manifest are unaffected. Both EF manifests declare it explicitly false until EF audit persistence lands. - IMessagesViewDataStore documents the precedence, paging and counting rules a persister must honour once one local result set unions failed and audited messages. - ServiceControl.Persistence.Tests.AuditCapable is a test-only persister that advertises audit support and delegates everything else to a real persister, so later pull requests can compose and start an audit-capable primary host before any shipped persister is one.
Adapts the standalone audit instance's receiving, enrichment, fault handling, forwarding and metrics into the primary project, wired to primary persistence and primary settings. The standalone audit executable is behaviorally unchanged. What the copy does differently: - Endpoint detection writes through IMonitoringIngestionUnitOfWork instead of sending RegisterNewEndpoint through IMessageSession, the same way ErrorProcessor does. IMessageSession and the ICommand overload on the enricher context are gone, so the runtime composes in a host with no NServiceBus endpoint. - Audit messages, saga snapshots and detected endpoints commit through one primary IIngestionUnitOfWork. - Transport output is dispatched through the receiving infrastructure's own dispatcher, matching the error ingestion scale-out work. Retry acknowledgements stay transport operations, because the acknowledgement queue is named by whichever instance issued the retry. - ITransportCustomization.CreateTransportInfrastructure takes a per receiver concurrency, so audit ingestion keeps its own concurrency in a host whose shared TransportSettings carries the primary endpoint's. - The meter is renamed to Particular.ServiceControl and the primary gains the three OpenTelemetry packages plus OtlpEndpointUrl. - The copied custom checks are renamed so they do not collide with the standalone audit instance reporting into this same primary. AuditComponent registers all of it only when the configured persister advertises SupportsAuditIngestion, so no shipped configuration changes behavior. The composition tests run against the audit capable test persister and assert what the host does and does not register, including that disabling ingestion stops only the receiver. Shared projects touched: ServiceControl.Transports gains an optional argument with the existing behavior as its default, so the audit executable's receiver is created exactly as before.
Adds --audit-ingestion-only to the command line and Help.txt, with the three checks the plan calls for. The host itself is not composed yet, so the command fails after its checks pass; no shipped persister advertises audit support, so in practice it fails on the storage check. - Storage that does not advertise SupportsAuditIngestion is rejected by name, driven by the manifest rather than by resolving optional services. RavenDB falls out of this without a special case. - The two ingestion only modes cannot be combined. Each queue gets its own worker pool so they can be scaled independently. - File system body storage is rejected unless ServiceControl/MessageBody/FileSystem/PathIsShared asserts the path is a shared mount. Nothing in the file system settings distinguishes a shared mount from a node local directory, so the operator has to say so. The same check now guards --error-ingestion-only, which PR #5801 documented as a known gap. No installer changes.
The two audit entry points that were remote only become ordinary scatter gather APIs over the new contracts, so a primary holding audit data answers them from its own storage and still merges in any remotes. Routes and authorization policies are unchanged: audit counts stay on error:messages:view and sagas/{id} stays on error:sagas:view, which is what a primary with an audit remote already serves them under.
- GetAuditCountsForEndpointApi drops the IMessagesViewDataStore it never used, along with the comment saying it would never be implemented here.
- GetSagaByIdApi stops deriving from ScatterGatherRemoteOnly, which nothing else used, so that base and its NoOpStore are gone.
- A persister without audit support falls back to an empty local source, registered with TryAdd after the persister, so those hosts behave exactly as before and the APIs still resolve.
LocalMessagesView.Merge puts the precedence, paging and counting rules in one place for any persister that returns failed and audited messages from one query: the failed row wins for a message that both failed and was audited, the total counts it once, and the local result is already one page. MessageViewComparer moves to the persistence project so a local merge can order rows the same way the scatter gather does.
IBodyStorage documents the arbitration order for a store holding both kinds of body, and AuditBodyKeyspace prefixes audit bodies so they do not collide with failed message bodies in the same store.
The audit capable test persister implements all of it, so the rules have a running implementation before the EF work starts.
Two primary owned capabilities depend on an audit remote existing today, and both break once a primary holds audit data itself. /api/connection is what ServicePulse and the Platform Connector plugin read to configure endpoints. With no audit remote it stops advertising MessageAudit.AuditQueue and SagaAudit.SagaAuditQueue, so endpoints cannot be told where to send audit or saga data at all. An audit capable primary now supplies the same two shapes the audit instance supplies, so nothing downstream sees a difference. SagaUpdatedHandler resolves the audit queue through the same IPlatformConnectionBuilder as before, reading either a remote's parsed JSON or the local provider's object, so a misdirected saga audit message is forwarded rather than failed. Audit throughput collection is likewise driven entirely by remotes. With local audit and no remotes, AuditQueues stays empty and the local audit and audit.log queues are counted as customer endpoints in the licensing throughput report, which is an accuracy defect rather than cosmetic, and AuditServicesData comes out blank. IAuditQuery gains an optional local source that contributes the local queue names, version, transport and retention alongside the remotes, so the existing platform endpoint exclusion, service metadata and connection diagnostics all work unchanged. Retention is reported exactly as configured. Where ServiceControl/AuditRetentionPeriod is unset the existing minimum retention gate warns, rather than this guessing a default: what null means is still an open item on the plan.
Adds the host --audit-ingestion-only runs: an audit receiver, the endpoint monitor it needs, and health endpoints, with no NServiceBus endpoint and none of the work a deployment may only do once. The component list is HeartbeatMonitoring and CustomChecks only. HeartbeatMonitoring is there because the audit enricher asks the endpoint monitor whether an endpoint is new before recording it, so without it every audited message writes a known endpoint upsert. CustomChecks is there so a stuck worker reports somewhere. EventLog and ExternalIntegrations are absent because audit ingestion raises no domain events and no integration events; the composition test states that, so a future registration forces a decision rather than slipping in. The ingestion only conditions scattered through the host become one Settings.IngestionOnly, so both modes agree on what they do not do: no endpoint, no retention sweep, no platform connection providers, no heartbeat checking. Ingestion health checks move to the components that host the ingestion they report on, so this host answers for audit-ingestion and not for error-ingestion, and the error ingestion only host is unchanged.
docs/audit-ingestion-in-the-primary.md covers the three deployment modes, the settings and their key collisions with a standalone audit instance, queue ownership and what ingestion only workers never provision, the body storage rule, the health endpoints, and the query behavior including the precedence, paging and counting rules. The primary logs a warning at startup when it both ingests audit messages and has audit remotes configured, because that is the shape most likely to hit the setting collisions the doc describes. An architecture test pins the boundary the plan depends on: the primary must not reference ServiceControl.Audit, which stays a standalone composition root. The packaging tests confirm the copied runtime ships inside the existing primary artifact, that the three new OpenTelemetry references change no deployment unit, and that the persisters folder is still exactly the four shipped storages.
…pid for scaled-out workers Scaled-out workers drain the same queue and share an instance name, so `service.name` alone cannot tell them apart on a dashboard. Every process now also reports `host.name` and `process.pid` by default, which makes a pool distinguishable without any operator configuration. `AddServiceControlInstance` centralises that logic and defers to the standard OpenTelemetry environment variables (`OTEL_SERVICE_NAME`, `OTEL_RESOURCE_ATTRIBUTES`) wherever an operator has already set them, including for `service.instance.id`, `host.name`, and `process.pid`. Both the metrics pipeline and the static bootstrap loggers call it, so every signal from a process carries the same identity. `ServiceControlMeters.Error` is renamed to `Primary` to reflect that the meter names the instance, not the subject: audit ingestion hosted in the primary publishes on the primary meter, and what a measurement is about is carried by the instrument prefix instead.
The implementation half of the audit-in-primary work. The hosting plan delivered contracts, runtime, settings and composition but nothing that stores an audit message; this covers the schema, partitioning, retention, body storage, search and queries needed to flip SupportsAuditIngestion on. Decisions settled by interview: - Audit tables join ServiceControlDbContext and the existing migration stream, with partitioning applied by raw SQL in a migration the way AddFullTextSearch already applies the GIN index. - created_on is the ingestion hour and part of the primary key, because PostgreSQL requires the partition key in every unique constraint. A redelivery across an hour boundary duplicates, which is stated rather than claimed away. - Audit bodies are keyed by hour so retention deletes a partition's bodies in one operation per store rather than one per message. - Full text search mirrors the error side rather than the spike's denormalised SearchableContent column. - Audit counts are an aggregate query, not a rollup table. - AuditRetentionPeriod defaults to 7 days when null. - The retention sweeper alone creates partitions, with a 48 hour lookahead and a custom check, since it is a single point of failure. - Retention takes a session scoped advisory lock, ported from the spike, so two misconfigured primaries cannot drop the same partition at once. The primary EF sweeper has never had one; the locks the spike wrote only ever existed in the standalone audit projects. The spike's insert-only KnownEndpoints table and its reconciler are not ported: the primary batch writer already dedupes endpoints in memory per batch and issues one insert-if-missing per distinct endpoint, and audit ingestion already routes through that same unit of work. SQL Server keeps the spike's batched deletes rather than partitioning, because a full text index cannot be aligned to a partition scheme.
First step of EF audit persistence. Schema only: nothing writes or reads these tables yet, and both manifests still declare SupportsAuditIngestion false, so no shipped instance changes behaviour. Ingestion is the hot path and reads are not, so the schema is shaped to make writes cheap and let queries pay. - AuditMessages, SagaSnapshots and FailedAuditImports join the existing ServiceControlDbContext and the per-provider migration stream, rather than getting a context of their own, so an ingestion batch can commit audit rows and known endpoints in one transaction. - On PostgreSQL the two audit tables are range partitioned on created_on. EF cannot express declarative partitioning and PostgreSQL cannot convert a table in place, so the migration clones each table with LIKE INCLUDING ALL after the columns and indexes are created. Cloning rather than hand writing the DDL keeps the definition owned by the entity model. - Both audit keys are a bigint identity, and exist only because a partitioned table's primary key must include the partition key and created_on alone is not unique. Nothing reads them back. Rows are plain inserts with no conflict clause, so neither table deduplicates and a redelivered message produces a second row. RavenDB does deduplicate, so this is a behaviour difference and not only a scale-out caveat. - Columns exist only where the read path filters, sorts, indexes or searches. InvokedSagas and OriginatesFromSaga get none, because InvokedSagasParser derives them from headers alone. - ServiceControl.Persistence.EFCore now references the saga audit project explicitly, since transitive project references are disabled there. The full text index is not here. SQL Server requires a single column unique KEY INDEX and the audit key is composite, and the indexed expression has to be written with the query expression it must match, so both land with the queries.
…igured AuditPartitioningSql previously generated table references without a schema qualifier, so the partitioning conversion ran against the connection's search path rather than the configured schema. This mirrors what FullTextSearchSql already does for the GIN index. - Convert PartitionTables and UnpartitionTables from methods to static fields holding the schema-neutral form, so SchemaStampingNpgsqlMigrationsSqlGenerator can recognise them by identity and re-render with the correct schema. - Add Rewrite and IsHandled to match the FullTextSearchSql pattern, and route SqlOperation through both in the generator. - Qualify table names with the schema only when one is set, leaving the schema-less form unchanged for the default case. - Regenerate both providers' AddAuditIngestion migrations to pick up the ExternalIntegrationDispatchRequests entity and a corrected EventLogItems table name, and mark FailingEndpointAddress required. - Update the schema-awareness test to exclude AuditPartitioningSql operations alongside FullTextSearchSql ones, and fix the partitioning acceptance test to qualify the table name before the regclass cast. - Remove the pending model changes test, which checked that hand-written DDL matched the entity model; that check is no longer valid after the new entity was added.
Two pipelines in one executable and one persister, segregated by folder rather than by project: the audit pipeline under Auditing/, the error pipeline under Operations/, shared ingestion helpers owned by neither, audit persistence under Implementation/Audit/, the audit host's API subset declared on the controllers, and host modes expressed as a profile rather than as mode checks inside components.
EndpointDetailsParser and ImportFailureCircuitBreaker are used by both the error and the audit pipeline, so they now live under Infrastructure/Ingestion and belong to neither. The audit metrics take the meter name from ServiceControlMeters rather than from the error pipeline's metrics class, which was its only remaining reason to reference Operations/. Each pipeline keeps its own metrics class since the instruments are named per pipeline.
EFIngestionUnitOfWork.Audit now returns a real unit of work. Recording buffers rows and queues external body writes, as the recoverability child does, and Complete writes them inside the transaction the failed message batch already used, after the failed message upsert, so a batch's audit rows and its known endpoints commit together. The transaction moved out of FailedMessageBatchWriter into the unit of work for that reason.
Inserts are plain multi-row statements from a per-provider dialect with no conflict clause: nothing deduplicates, and a redelivered audit message or saga audit message produces a second row, which the tests assert because RavenDB behaves differently.
Every row of a batch carries the hour the batch started in. That hour is the PostgreSQL partition key and the first segment of the external body key, audit/{hour}/{unique message id}, which is what will let retention drop an hour's bodies with one prefix delete. The file system store creates that directory on write.
Partition provisioning lands here rather than with retention because on PostgreSQL nothing can be inserted into an hour nobody provisioned: the migrator provisions the hour before now through the 48 hour lookahead after migrating, through IAuditPartitionManager, which is a no-op on SQL Server.
The retention sweeper gains an audit pass. Audit rows expire an hour at a time once the whole hour is behind the cutoff: the hour's external bodies go first, by one prefix delete per store, then the hour's rows. On PostgreSQL that is a DROP TABLE of the hour's partitions; on SQL Server, which does not partition, it is a batched delete paced like the failed message sweep. The same pass keeps the provisioned partition window ahead of the clock, because the sweeper is the only host that issues DDL. AuditRetentionPeriod defaults to seven days, the value the management utility and the container image apply. Every sweep now runs under a session scoped database lock, ported from the audit spike: pg_try_advisory_lock on PostgreSQL, sp_getapplock on SQL Server, on a dedicated unpooled connection, with a zero timeout so a host that cannot take it skips the pass. The resource name carries the configured schema, so instances sharing a database through different schemas do not take turns. This puts error and event log retention under the lock for the first time, deliberately: two hosts dropping the same partition is unforgiving in a way two hosts running the same batched delete is not. A custom check on the PostgreSQL persister fails when the newest provisioned partition ends less than twelve hours ahead, so a sweeper that has stopped provisioning is visible before ingestion stops.
Each message view is one statement over both tables: the failed and audit branches project to a common row, each carries its own sort and limit, and the union is sorted and paged once more. The branch limits are what make it fast, measured on PostgreSQL 16 with three million audit rows over 168 partitions: the same union with the sort outside made the planner hash the anti-join and scan every row, 1.2 seconds for page one, where this shape merges two index-ordered scans in 5 ms and takes 8 ms at page one hundred. Precedence is the rule the scatter gather has always applied, a failed row wins whatever its status, as an anti-join on the audit branch alone. The total is capped, because an exact count is linear in the audit table. Audit counts group the endpoint's non-system messages by processing day over the last thirty days, with the RavenDB persister's zero for an endpoint that only ever sent. Saga history pages a saga's snapshots newest first. Body arbitration gains its third step, the audit row by unique message id, with the external key composed in memory because a Guid rendered by SQL Server is upper case. The audit full-text index mirrors the failed one on both providers and is pinned to the query expression by the same test. On SQL Server it keys on a unique index over the identity column, since a full-text key index must be a single column and the audit primary key is composite.
The audit counterpart of the failed error import store: keyed by FailedAuditImport.DeriveKey so a poison message produces one row however many workers attempt it, bodies over the cap spilled to external storage under the failedauditimport prefix, replay oldest first with failures left in place, and no retention. The reimport command already drives the contract, so it needs no change.
Both manifests now set SupportsAuditIngestion, so a primary on either persister hosts the audit runtime: the receiver, saga audit, failed audit imports, the local audit queries and audit retention. The acceptance suites run against the real persisters, which retires the audit capable test persister that stood in for them, along with the scenarios that could only exist while a relational persister lacked audit support.
A primary whose audit load should not share its database can move audit to a dedicated one. ServiceControl.exe --audit-instance is the owner of that database: it ingests the audit queue, serves the primary API so the primary's scatter gather can list it under RemoteInstances, sweeps audit retention, and with --setup provisions the database, the audit queue and body storage. The primary is told the data is elsewhere through ServiceControl/AuditDataLocation=Remote, which turns off its audit receiver, its local audit queries, its audit retention pass and its partition provisioning, and replaces the old warning about remotes plus local ingestion with a validation error. Hosts on the audit database report to the primary the way the RavenDB audit instance does, custom check results and detected endpoints sent to ServiceControl/ServiceControlQueueAddress, through a send only endpoint built from the transport customization's audit profile. A worker with that setting reports too, so scaling out the audit host needs no new flag. Components no longer branch on the mode name. Settings.Host is a profile derived once from the flags and each component asks for the capability it needs, which the third host mode made worth having. Workers gain a schema probe that refuses to start against a database the owner has not migrated yet, naming setup instead of failing on the first insert.
The shipped hosting document gains the shared and dedicated audit database topologies with the processes each runs, the AuditDataLocation and ServiceControlQueueAddress settings, the seven day retention default, the hourly retention model and its lock, and the database-side message view with its precedence and capped total. The hosting plan's statement that there is no separate audit HTTP service is marked superseded.
johnsimons
force-pushed
the
john/audit_ef_9
branch
from
September 18, 2026 03:39
63aee13 to
3279a7e
Compare
rbev
reviewed
Sep 18, 2026
rbev
left a comment
rbev
left a comment
Contributor
There was a problem hiding this comment.
Design doc comments/questions
| not gain combined hosting, and keeps its own executable, settings, API and installers. | ||
|
|
||
| The SQL Server and PostgreSQL persisters advertise audit support, so a primary on either ingests | ||
| the audit queue by default. On RavenDB the audit component registers nothing and behavior is |
Contributor
There was a problem hiding this comment.
I think this ingestion should be an explicit opt-in (defaulted on for new installs) since a migrating user might be caught off guard by their error instance suddenly consuming their audits.
| and a standalone audit instance sharing one environment file therefore collide on | ||
| `INGESTAUDITMESSAGES`, `AUDITRETENTIONPERIOD`, `FORWARDAUDITMESSAGES` and `SERVICEBUS_AUDITQUEUE`. | ||
|
|
||
| That combination is unsupported. A primary that ingests audit into its own database and also lists |
Contributor
There was a problem hiding this comment.
If we aren't migrating audit then doesn't this break the migration path into a single Sql server from Error+Audit on raven?
How would you go from this:
Error -> RavenEmbedded
Audit -> RavenEmbedded
To this, if the error refuses to start with remotes?
Error With Audit ingestion -> Sql
Audit (No ingestion, decomission when empty) -> RavenEmbedded
| any worker; a worker started against a database the owner has not migrated refuses to start and says | ||
| so, rather than failing on its first write. | ||
|
|
||
| The primary and a dedicated audit database run the same migrations, so the owners are upgraded |
Contributor
There was a problem hiding this comment.
The paragraph above says that it guards against startup and never upgrades the db, then this paragraph says they can be upgraded in any order. Seems like these are incompatible constraints/requirements.
| Audit and failed message bodies share one store, and each owns a prefixed keyspace, so an edited | ||
| message's failed body and its audited body do not collide. Audit bodies are keyed | ||
| `audit/{ingestion hour}/{unique message id}`, which is what lets retention drop an hour's bodies in | ||
| one operation. `IBodyStorage.TryFetch` resolves in a fixed order: failed message by |
Contributor
There was a problem hiding this comment.
It seems weird that the audit and error bodies are merged together at read time despite being explicitly stored separately
| one operation. `IBodyStorage.TryFetch` resolves in a fixed order: failed message by | ||
| `UniqueMessageId`, then failed message by `MessageId`, then audit message by `UniqueMessageId`. | ||
|
|
||
| Hosts on one database share one body store: the audit host and its workers share the audit store, |
Contributor
There was a problem hiding this comment.
Do we need to allow a single primary instance do audit/error on different databases? It seems like it's a bit of an uneccessary limitation/cost to require a separate instance to allow that scenario.
In that case would they still share a body storage?
| settings can detect, so both ingestion only modes refuse to start unless | ||
| `ServiceControl/MessageBody/FileSystem/PathIsShared` asserts it. | ||
|
|
||
| ## Health endpoints |
Contributor
There was a problem hiding this comment.
we should answer the question about how these can appear in the platform health page, that currently relies on the remotes list.
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
2 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.
This design document outlines the architectural strategy and work plan for moving audit ingestion into the primary ServiceControl process for SQL Server and PostgreSQL persistence, including support for scaled-out ingestion-only workers.