-
Notifications
You must be signed in to change notification settings - Fork 682
Fix dashboard log parsing in host #5425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,79 @@ | ||||||
// Licensed to the .NET Foundation under one or more agreements. | ||||||
// The .NET Foundation licenses this file to you under the MIT license. | ||||||
|
||||||
using System.Diagnostics.CodeAnalysis; | ||||||
using System.Globalization; | ||||||
using System.Text.RegularExpressions; | ||||||
|
||||||
namespace Aspire.Dashboard.ConsoleLogs; | ||||||
|
||||||
internal static partial class TimestampParser | ||||||
{ | ||||||
private static readonly Regex s_rfc3339RegEx = GenerateRfc3339RegEx(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to minimize changes here and for 8.2 backport. I'll make regex changes in #5398 |
||||||
|
||||||
public static bool TryParseConsoleTimestamp(string text, [NotNullWhen(true)] out TimestampParserResult? result) | ||||||
{ | ||||||
var match = s_rfc3339RegEx.Match(text); | ||||||
|
||||||
if (match.Success) | ||||||
{ | ||||||
var span = text.AsSpan(); | ||||||
var timestamp = span[match.Index..(match.Index + match.Length)]; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can just be:
Suggested change
|
||||||
|
||||||
ReadOnlySpan<char> content; | ||||||
if (match.Index + match.Length >= span.Length) | ||||||
{ | ||||||
content = ""; | ||||||
} | ||||||
else | ||||||
{ | ||||||
content = span[(match.Index + match.Length)..]; | ||||||
|
||||||
// Trim whitespace added by logging between timestamp and content. | ||||||
if (char.IsWhiteSpace(content[0])) | ||||||
{ | ||||||
content = content.Slice(1); | ||||||
} | ||||||
} | ||||||
|
||||||
result = new(content.ToString(), DateTimeOffset.Parse(timestamp.ToString(), CultureInfo.InvariantCulture)); | ||||||
return true; | ||||||
} | ||||||
|
||||||
result = default; | ||||||
return false; | ||||||
} | ||||||
|
||||||
// Regular Expression for an RFC3339 timestamp, including RFC3339Nano | ||||||
// | ||||||
// Example timestamps: | ||||||
// 2023-10-02T12:56:35.123456789Z | ||||||
// 2023-10-02T13:56:35.123456789+10:00 | ||||||
// 2023-10-02T13:56:35.123456789-10:00 | ||||||
// 2023-10-02T13:56:35.123456789Z10:00 | ||||||
// 2023-10-02T13:56:35.123456Z | ||||||
// 2023-10-02T13:56:35Z | ||||||
// | ||||||
// Explanation: | ||||||
// ^ - Starts the string | ||||||
// (?:\\d{4}) - Four digits for the year | ||||||
// - - Separator for the date | ||||||
// (?:0[1-9]|1[0-2]) - Two digits for the month, restricted to 01-12 | ||||||
// - - Separator for the date | ||||||
// (?:0[1-9]|[12][0-9]|3[01]) - Two digits for the day, restricted to 01-31 | ||||||
// [T ] - Literal, separator between date and time, either a T or a space | ||||||
// (?:[01][0-9]|2[0-3]) - Two digits for the hour, restricted to 00-23 | ||||||
// : - Separator for the time | ||||||
// (?:[0-5][0-9]) - Two digits for the minutes, restricted to 00-59 | ||||||
// : - Separator for the time | ||||||
// (?:[0-5][0-9]) - Two digits for the seconds, restricted to 00-59 | ||||||
// (?:\\.\\d{1,9}) - A period and up to nine digits for the partial seconds | ||||||
// Z - Literal, same as +00:00 | ||||||
// (?:[Z+-](?:[01][0-9]|2[0-3]):(?:[0-5][0-9])) - Time Zone offset, in the form ZHH:MM or +HH:MM or -HH:MM | ||||||
// | ||||||
// Note: (?:) is a non-capturing group, since we don't care about the values, we are just interested in whether or not there is a match | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using Consider also adding |
||||||
[GeneratedRegex("^(?:\\d{4})-(?:0[1-9]|1[0-2])-(?:0[1-9]|[12][0-9]|3[01])T(?:[01][0-9]|2[0-3]):(?:[0-5][0-9]):(?:[0-5][0-9])(?:\\.\\d{1,9})?(?:Z|(?:[Z+-](?:[01][0-9]|2[0-3]):(?:[0-5][0-9])))?")] | ||||||
Comment on lines
+57
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The duplication here can be avoided by using Something like: [GeneratedRegex("""
^ # Starts the string
\d{4} # Four digits for the year
- # Separator for the date
(0[1-9]|1[0-2]) # Two digits for the month, restricted to 01-12
- # Separator for the date
(0[1-9]|[12][0-9]|3[01]) # Two digits for the day, restricted to 01-31
[T ] # Literal, separator between date and time, either a T or a space
([01][0-9]|2[0-3]) # Two digits for the hour, restricted to 00-23
: # Separator for the time
([0-5][0-9]) # Two digits for the minutes, restricted to 00-59
: # Separator for the time
[0-5][0-9] # Two digits for the seconds, restricted to 00-59
\.\d{1,9} # A period and up to nine digits for the partial seconds
Z # Literal, same as +00:00
[Z+-]([01][0-9]|2[0-3]):[0-5][0-9] # Time Zone offset, in the form ZHH:MM or +HH:MM or -HH:MM
""",
RegexOptions.ExplicitCapture | RegexOptions.InvariantCulture | RegexOptions.IgnorePatternWhitespace)] |
||||||
private static partial Regex GenerateRfc3339RegEx(); | ||||||
|
||||||
public readonly record struct TimestampParserResult(string ModifiedText, DateTimeOffset Timestamp); | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,143 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Globalization; | ||
using System.Text.Json; | ||
using System.Threading.Channels; | ||
using Aspire.Hosting.Dashboard; | ||
using Aspire.Hosting.Dcp; | ||
using Microsoft.Extensions.Configuration; | ||
using Microsoft.Extensions.Hosting; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.Extensions.Logging.Abstractions; | ||
using Microsoft.Extensions.Logging.Testing; | ||
using Microsoft.Extensions.Options; | ||
using Xunit; | ||
|
||
namespace Aspire.Hosting.Tests.Dashboard; | ||
|
||
public class DashboardLifecycleHookTests | ||
{ | ||
[Theory] | ||
[MemberData(nameof(Data))] | ||
public async Task WatchDashboardLogs_WrittenToHostLoggerFactory(string logMessage, string expectedMessage, string expectedCategory, LogLevel expectedLevel) | ||
{ | ||
// Arrange | ||
var testSink = new TestSink(); | ||
var factory = LoggerFactory.Create(b => | ||
{ | ||
b.SetMinimumLevel(LogLevel.Trace); | ||
b.AddProvider(new TestLoggerProvider(testSink)); | ||
}); | ||
var logChannel = Channel.CreateUnbounded<WriteContext>(); | ||
testSink.MessageLogged += c => logChannel.Writer.TryWrite(c); | ||
|
||
var resourceLoggerService = new ResourceLoggerService(); | ||
var resourceNotificationService = new ResourceNotificationService(NullLogger<ResourceNotificationService>.Instance, new TestHostApplicationLifetime()); | ||
var configuration = new ConfigurationBuilder().Build(); | ||
var hook = new DashboardLifecycleHook( | ||
configuration, | ||
Options.Create(new DashboardOptions { DashboardPath = "test.dll" }), | ||
NullLogger<DistributedApplication>.Instance, | ||
new TestDashboardEndpointProvider(), | ||
new DistributedApplicationExecutionContext(DistributedApplicationOperation.Run), | ||
resourceNotificationService, | ||
resourceLoggerService, | ||
factory); | ||
|
||
var model = new DistributedApplicationModel(new ResourceCollection()); | ||
await hook.BeforeStartAsync(model, CancellationToken.None); | ||
|
||
await resourceNotificationService.PublishUpdateAsync(model.Resources.Single(), s => s); | ||
|
||
await foreach (var item in resourceLoggerService.WatchAnySubscribersAsync()) | ||
{ | ||
if (item.Name == KnownResourceNames.AspireDashboard && item.AnySubscribers) | ||
{ | ||
break; | ||
} | ||
} | ||
|
||
// Act | ||
var dashboardLogger = resourceLoggerService.GetLogger(KnownResourceNames.AspireDashboard); | ||
dashboardLogger.LogError(logMessage); | ||
|
||
// Assert | ||
var logContext = await logChannel.Reader.ReadAsync(); | ||
Assert.Equal(expectedCategory, logContext.LoggerName); | ||
Assert.Equal(expectedMessage, logContext.Message); | ||
Assert.Equal(expectedLevel, logContext.LogLevel); | ||
} | ||
|
||
public static IEnumerable<object[]> Data() | ||
{ | ||
var timestamp = new DateTime(2001, 12, 29, 23, 59, 59, DateTimeKind.Utc); | ||
var message = new DashboardLogMessage | ||
{ | ||
LogLevel = LogLevel.Error, | ||
Category = "TestCategory", | ||
Message = "Hello world", | ||
Timestamp = timestamp.ToString(KnownFormats.ConsoleLogsTimestampFormat, CultureInfo.InvariantCulture), | ||
}; | ||
var messageJson = JsonSerializer.Serialize(message, DashboardLogMessageContext.Default.DashboardLogMessage); | ||
|
||
yield return new object[] | ||
{ | ||
$"{DateTime.UtcNow.ToString(KnownFormats.ConsoleLogsTimestampFormat, CultureInfo.InvariantCulture)} {messageJson}", | ||
"Hello world", | ||
"Aspire.Hosting.Dashboard.TestCategory", | ||
LogLevel.Error | ||
}; | ||
yield return new object[] | ||
{ | ||
$"{DateTime.UtcNow.ToString(KnownFormats.ConsoleLogsTimestampFormat, CultureInfo.InvariantCulture)}{messageJson}", | ||
"Hello world", | ||
"Aspire.Hosting.Dashboard.TestCategory", | ||
LogLevel.Error | ||
}; | ||
yield return new object[] | ||
{ | ||
messageJson, | ||
"Hello world", | ||
"Aspire.Hosting.Dashboard.TestCategory", | ||
LogLevel.Error | ||
}; | ||
|
||
message = new DashboardLogMessage | ||
{ | ||
LogLevel = LogLevel.Critical, | ||
Category = "TestCategory.TestSubCategory", | ||
Message = "Error message", | ||
Exception = new InvalidOperationException("Error!").ToString(), | ||
Timestamp = timestamp.ToString(KnownFormats.ConsoleLogsTimestampFormat, CultureInfo.InvariantCulture), | ||
}; | ||
messageJson = JsonSerializer.Serialize(message, DashboardLogMessageContext.Default.DashboardLogMessage); | ||
|
||
yield return new object[] | ||
{ | ||
messageJson, | ||
$"Error message{Environment.NewLine}System.InvalidOperationException: Error!", | ||
"Aspire.Hosting.Dashboard.TestCategory.TestSubCategory", | ||
LogLevel.Critical | ||
}; | ||
} | ||
|
||
private sealed class TestDashboardEndpointProvider : IDashboardEndpointProvider | ||
{ | ||
public Task<string> GetResourceServiceUriAsync(CancellationToken cancellationToken = default) | ||
{ | ||
throw new NotImplementedException(); | ||
} | ||
} | ||
|
||
private sealed class TestHostApplicationLifetime : IHostApplicationLifetime | ||
{ | ||
public CancellationToken ApplicationStarted { get; } | ||
public CancellationToken ApplicationStopped { get; } | ||
public CancellationToken ApplicationStopping { get; } | ||
|
||
public void StopApplication() | ||
{ | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is mostly a copy and paste from what is in the dashboard. There will be duplicate copies after this PR but this will be cleaned up by #5398. I did this to minimize changes.