Event log persistence for the SQL Server and PostgreSQL persisters by warwickschroeder · Pull Request #5655 · Particular/ServiceControl

warwickschroeder · 2026-07-28T00:49:10Z

The event log is the list ServicePulse shows on its Events page. It worked on RavenDB and threw NotImplementedException on both EF Core persisters, so nothing could be written or read. This implements it.

Both EF persisters still log "is still under development and is not ready for use" at startup, so no customer is affected today.

What changed

EventLogItemEntity, its configuration, and one migration per provider. EventLogDataStore now implements both methods.
The write moved to the interface that owns it. AuditEventLogWriter called IErrorMessageDataStore.StoreEventLogItem while IEventLogDataStore.Add sat unused. Now it calls Add, and the stray member is gone from IErrorMessageDataStore and both implementations.
Event log items expire on the EF side too. RetentionSweeper sweeps them on its existing hourly timer against a separate EventsRetentionPeriod, independent of ErrorRetentionPeriod.
Conditional GET. GetEventLogItems takes an optional knownVersion. On a match the persister skips the page query and EventLogApiController answers 304, with ETag and Total-Count still sent.
knownVersion drops a query per poll on the EF persisters. A read is three queries: count, newest RaisedAt, then the page. When the caller's version matches, the page query is skipped and the two that remain are satisfied by IX_EventLogItems_RaisedAt_Id without reading row data. ServicePulse polls this every five seconds, so on an idle instance that is the common case rather than the exception. RavenDB saves no database work, because its version is the query's own ResultEtag and does not exist until the query has run; there the saving is only the serialised response body.
IEventLogDataStore now documents retention, which was previously invisible at the seam despite each persister enforcing it differently.

Tests

21 new tests plus 3 added to RetentionSweepTests, and one acceptance test for the 304. The 14 in EventLogDataStoreTests are the valuable ones: they live in the shared root of ServiceControl.Persistence.Tests, so they run as conformance tests against RavenDB, SQL Server and PostgreSQL. Round-tripping, ordering, paging boundaries and all four knownVersion cases have to behave identically on all three.

johnsimons · 2026-07-28T01:29:43Z

+    public long Id { get; set; }
+
+    // The API-visible identity, assigned by EventLogMappingDefinition as
+    // "EventLogItem/{Category}/{EventType}/{guid}"


This feels a lot like Raven Ids!

It sure is, which is why I havent made this the PK and introduced a long for that. Ill take another look into if we need this field at all in EF.

I think we should keep the API-visible identity for each event log item the same. The database ID field used by the persistence doesn't need to be this though.

I'm not sure I understand what you mean

This commit should move the Id generation into the persistence seams and out of the abstraction layer.

johnsimons · 2026-07-28T01:32:48Z

+
+    public DateTime RaisedAt { get; set; }
+
+    public List<string> RelatedTo { get; set; } = [];


is EF smart to serialize this as xml/json?

In SQL server, yes. It maps to a json array string. In Postrgres, maps to a a native text[] type

I just looked in SP and it seems this is the only place we are using the "related_to" prop, see https://github.com/Particular/ServicePulse/blob/5f0ede166f9a1209a288659d4f8f0c59067525ea/src/Frontend/src/components/EventLogItem.vue#L30-L33 and it seems we are only looking at the first item!

Yeh looks that way, only the leading /message/ link. Ill add a board card to discuss.

johnsimons · 2026-07-28T01:57:20Z

+// rather than the shape of MessageFailed.
+class EventLogWriterTests : PersistenceTestBase
+{
+    public EventLogWriterTests() => RegisterServices = services => services.AddSignalR();


Why do we need SignalR?

So, looks like SignalR is still fully wired up and broadcasting 25 domain event types in ServiceControl... just to nobody it seems!

ServiceControl/src/ServiceControl/Infrastructure/WebApi/HostApplicationBuilderExtensions.cs

Line 42 in ecbad0a

var signalR = builder.Services.AddSignalR();

I did a search in ServicePulse and ServiceInsight and neither are clients of it. Integration events is something seperate and dont use SignalR. I dont know what else to check to make sure this isnt being used. Ill add something to our board to confirm and potentially clean up.

If there's no clients signalR doesn't actually "broadcast" anything, the only cost would be if there's a distributed backplane set up but looking at that it's only using the in-memory one.

The only product that consumed SignalR was SP a long long time ago, so I think is time to remove it

Once (if) SignalR is removed, I will remove it from this test too

johnsimons · 2026-07-28T02:01:48Z

+            if (results is null)
+            {
+                return StatusCode((int)HttpStatusCode.NotModified);
+            }
+
+            return Ok(results);


This feels weird.
My thought is that we have middleware that handles it for us

We do have middleware for this, but the problem with that is that it was still querying the database. This skips the query if the version tag matches the header

rbev · 2026-07-28T05:20:04Z

+        {
+            var query = dbContext.EventLogItems.AsNoTracking();
+
+            var total = await query.LongCountAsync();


are these data pages really so big that doing two database roundtrips is faster than just going to get the data?

To reliably create a version id, we need to count all rows. There could be a lot of rows in the event logs. Maybe this would work? AI recons that would be 1 round trip, matching (and improving) what Raven is currently doing as its 1 trip, but also returning data.

var stats = await query .GroupBy(_ => 1) .Select(g => new { Total = g.LongCount(), Newest = g.Max(e => (DateTime?)e.RaisedAt) }) .FirstOrDefaultAsync();

Yep that worked. 1 round trip now.

rbev · 2026-07-28T05:27:14Z


-            return results;
+            // Null items means "you already hold this version"
+            if (results is null)


Suggested change

if (results is null)

if (results is null && totalCount > 0)

This suggestion doesnt look right. results being null means that the If-None-Match matched the current version of the data. I dont think we want to check for actual data

…QL Server, and tests

…og items with versioning support

…ed items

…em retention and cache validation behavior

- Seperate EventLogItem into a read/dto, and write model. The persistence now takes care of setting an Id, not the application layer - Add tests

…e related tests

internalautomation Bot assigned warwickschroeder Jul 28, 2026

warwickschroeder requested review from abparticular, johnsimons and rbev July 28, 2026 01:05

rbev approved these changes Jul 28, 2026

View reviewed changes

Comment thread src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs Outdated

Comment thread src/ServiceControl.Persistence.RavenDB/EventLogDataStore.cs

johnsimons reviewed Jul 28, 2026

View reviewed changes

rbev reviewed Jul 28, 2026

View reviewed changes

johnsimons mentioned this pull request Jul 28, 2026

Remove SignalR #5659

Open

warwickschroeder force-pushed the tf832-sql-eventlogs branch from bddff53 to 1233101 Compare July 28, 2026 09:20

warwickschroeder added 10 commits July 29, 2026 16:22

Add EventLogItems entity and related configurations, migrations for S…

ce9d363

…QL Server, and tests

Update sql migrations. Add postgres migrations

37aa628

Implement EventLogDataStore methods for adding and retrieving event l…

f48259f

…og items with versioning support

Add event log retention period and implement sweeping logic for expir…

9543109

…ed items

Enhance documentation for IEventLogDataStore interface, clarifying it…

87b5f53

…em retention and cache validation behavior

Add EnableIntegratedServicePulse setting to App.config

c483efe

Review changes

96a41fe

Regenerate migrations after rebase

fcf2ddd

- Remove RavenDB specifics into the RavenDB persistence seam.

38a3b90

- Seperate EventLogItem into a read/dto, and write model. The persistence now takes care of setting an Id, not the application layer - Add tests

Update EventLogItems migration for PostgreSQL and SQL Server

8ce240f

warwickschroeder force-pushed the tf832-sql-eventlogs branch from fb53f38 to 8ce240f Compare July 29, 2026 08:34

warwickschroeder marked this pull request as draft July 29, 2026 11:35

warwickschroeder added 2 commits July 29, 2026 20:29

Refactor GetEventLogItems method to return QueryResult type and updat…

78bb9bd

…e related tests

include HighestId in etag versioning and add unit test

b9d1b7c


		public DateTime RaisedAt { get; set; }

		public List<string> RelatedTo { get; set; } = [];

Uh oh!

Conversation

warwickschroeder commented Jul 28, 2026

What changed

Tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warwickschroeder Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warwickschroeder Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warwickschroeder Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

warwickschroeder Jul 28, 2026 •

edited

Loading

warwickschroeder Jul 29, 2026 •

edited

Loading

warwickschroeder Jul 28, 2026 •

edited

Loading