feat: add UTC timestamps to all console log entries - #3709
feat: add UTC timestamps to all console log entries#3709Jerry Nixon (JerryNixon) with Copilot wants to merge 12 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to make console logs easier to correlate by prepending an ISO 8601 UTC timestamp (millisecond precision) to console output emitted by both the Service host and the CLI custom logger.
Changes:
- Updated Service logging to use the console “simple” formatter with UTC timestamp settings (including MCP stdio stderr routing for the startup logger factory).
- Updated the CLI custom console logger to prepend a UTC timestamp before the abbreviated log level label.
- Adjusted CLI unit tests for the new timestamp-prefixed output (but the updated assertion is now too permissive).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Service/Program.cs | Switches console logging to include UTC timestamps; MCP stdio path continues to keep stdout clean. |
| src/Cli/CustomLoggerProvider.cs | Prepends UTC timestamps to CLI log prefixes for both standard and MCP stdio modes. |
| src/Cli.Tests/CustomLoggerTests.cs | Updates assertions to account for timestamps in log output. |
| // Apply colors so the abbreviation matches the visual style of engine logs. | ||
| // try/finally guarantees the original colors are restored even if Write throws, | ||
| // otherwise the console would be left tinted (e.g. red on error) for subsequent output. | ||
| string mcpTimestamp = DateTime.UtcNow.ToString(UtcTimestampFormat); |
| } | ||
|
|
||
| TextWriter writer = logLevel >= LogLevel.Error ? Console.Error : Console.Out; | ||
| string timestamp = DateTime.UtcNow.ToString(UtcTimestampFormat); |
| Assert.IsTrue(actual.Contains(expectedPrefix), | ||
| $"Expected output to contain '{expectedPrefix}' but got: '{actual}'"); |
| logging.AddSimpleConsole(options => | ||
| { | ||
| options.TimestampFormat = "yyyy-MM-dd'T'HH:mm:ss.fff'Z' "; | ||
| options.UseUtcTimestamp = true; | ||
| }); |
Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Done in d7fbf1b. Added Migrated diagnostics:
Against the acceptance criteria:
Also added |
…d diagnostics Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Done in 0a43f4c. Writing the requested Shared timestamp assertion (in both test projects) verifies the prefix: parses with New/updated coverage:
Mutation-checked: deleting the MCP timestamp fails both MCP tests, and reverting the |
|
The service console paths still do not satisfy the culture-invariant timestamp requirement. The formatter configuration at: uses I reproduced the actual .NET 10 formatter output with the current format:
Therefore, these paths do not consistently emit the required Gregorian Please use a console formatter that formats the UTC timestamp explicitly with Please also add regression coverage that runs both |
|
The direct-console inventory is still incomplete. Two log-like diagnostics remain in the production Aspire executable: Both use bare Please route these messages through the centralized timestamped logging path, or explicitly narrow and document the PR's scope if Aspire AppHost output is intentionally excluded from the “all console log entries” contract. If it remains in scope, please add coverage confirming that these diagnostics use the same invariant UTC millisecond prefix. |
…tter Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Fixed in 7d24499. Reproduced first — with the previous Added
Regression coverage in Solution builds with 0 warnings and |
…e inventory Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Kept in scope and fixed in 5a7e038. Both AppHost diagnostics now go through One wrinkle worth noting: the Aspire SDK rewrites For coverage: AppHost is top-level statements in an executable that builds and runs a distributed application, so invoking it from a test isn't practical. Instead of asserting the prefix a fourth time, I added
That is the load-bearing link: the BootstrapLogger tests (including the One change you should look at, outside what you asked for. Line 58 was Solution builds with 0 warnings, |
|
The custom formatter needs to preserve In .NET 10, I reproduced both behaviors by invoking this formatter with the same
The built-in |
|
Referencing the full Config project just to reuse Before this reference was added, AppHost built without warnings. At the current head, both the AppHost build and full solution build report
This couples AppHost to Config's Key Vault/Azure dependency graph and causes older Azure assemblies to win dependency resolution for the executable. Please avoid taking the full Config dependency for this small logging utility—for example, move the dependency-light timestamp/bootstrap logging functionality into an appropriate shared project, or use an AppHost-local logging path—and restore a warning-free AppHost build. |
|
…nsole formatter Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Fixed in 360c058. Confirmed against the .NET 10 Restructured Tests:
Mutation-checked: removing the buffered branch fails all three rows. One extra fix. Comparing against the built-in surfaced a second divergence I'd introduced: it runs message, exception, and category through Solution builds with 0 warnings, |
…nsole guard Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Fixed in 59dd5a1 by removing the Config reference entirely.
Verification that the coupling is actually gone: the first build after switching the reference failed with One thing I should flag: I was not able to reproduce the Note:
Both addressed in 59dd5a1. 1. 2. PostgreSQL connection-string behavior. Added To be clear about its limits: this verifies the guard/diagnostic pairing, not the downstream Mutation-checked both: re-inverting the postgres guard fails the new test, and adding a bare Note: |
Why make this change?
Console log output lacks timestamps, making it difficult to correlate events or determine when entries occurred — especially under high request volume.
What is this change?
Prepends an ISO 8601 UTC timestamp with millisecond precision to every console log entry:
src/Service/Program.cs— ReplacesAddConsole()withAddSimpleConsole(TimestampFormat, UseUtcTimestamp)in bothGetLoggerFactoryForLogLevel(startup logger) andCreateHostBuilder.ConfigureLogging(web host logger). MCP stdio path additionally usesServices.Configure<ConsoleLoggerOptions>for stderr routing, keeping a single registered provider.src/Cli/CustomLoggerProvider.cs— PrependsDateTime.UtcNow.ToString(UtcTimestampFormat)before the abbreviated level label in the CLI's custom console logger (both standard and MCP stdio paths). Timestamp format extracted to a named constant.src/Cli.Tests/CustomLoggerTests.cs— UpdatesLogOutput_UsesAbbreviatedLogLevelLabelsassertion fromStartsWithtoContainssince the timestamp now precedes the level label.How was this tested?
Sample Request(s)
No REST/GraphQL/CLI request changes — output-only behavioral change visible when running
dab start.