Skip to content
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

More precise timestamps in structured log view of dashboard #3127

Closed
KalleOlaviNiemitalo opened this issue Mar 24, 2024 · 14 comments · Fixed by #3202 or #3215
Closed

More precise timestamps in structured log view of dashboard #3127

KalleOlaviNiemitalo opened this issue Mar 24, 2024 · 14 comments · Fixed by #3202 or #3215

Comments

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Mar 24, 2024

In Aspire 8.0.0-preview.4.24156.9, the "Timestamp" column of the "Structured logs" page of the dashboard shows only one-second precision, e.g. "19.01.29". I'd like to see timestamps with more precision, like "19.08.17,7547722". That would let me see how much time passed between consecutive log entries, even if they occurred during the same second. In OpenTelemetry Protocol, LogRecord appears to support nanosecond precision.

I found a related issue #115 but that one seems to concern how the Dashboard configures the console loggers of other processes, rather than how it displays the timestamps it has received over OTLP.

@leslierichardson95
Copy link

@JamesNK what's your take on this scenario?

@JamesNK
Copy link
Member

JamesNK commented Mar 26, 2024

I think showing up to 3 decimal places in the UI is good. That's what most people want, most of the time, while keeping a clean UI. However, I think the tooltip can be improved to show the full precision.

Current:

image

@KalleOlaviNiemitalo
Copy link
Author

Aspire 8.0.0-preview.4.24156.9 already attempted to display milliseconds in the timestamp column:

<TemplateColumn Title="@Loc[nameof(Dashboard.Resources.StructuredLogs.StructuredLogsTimestampColumnHeader)]" TooltipText="@(context => FormatHelpers.FormatDateTime(context.TimeStamp, true, CultureInfo.CurrentCulture))" Tooltip="true">
@FormatHelpers.FormatTimeWithOptionalDate(context.TimeStamp, includeMilliseconds: true)
</TemplateColumn>

I think that did not work because it searched for the seconds field using the regular expression (:ss|:s):

[GeneratedRegex("(:ss|:s)")]
private static partial Regex MatchSecondsInTimeFormatPattern();

and CultureInfo.CurrentCulture.DateTimeFormat.LongTimePattern actually was "H.mm.ss", which does not include a colon, so FormatHelpers did not know where they should insert the milliseconds.

Now on the "main" branch, FormatHelpers are still using colons in the regular expression:

[GeneratedRegex("(:ss|:s)")]
private static partial Regex MatchSecondsInTimeFormatPattern();

Because of that, I suspect it still does not work.

@JamesNK
Copy link
Member

JamesNK commented Mar 27, 2024

What culture are you using? Can you show a screenshot of what you see?

@KalleOlaviNiemitalo
Copy link
Author

The culture is "fi-FI".

@KalleOlaviNiemitalo
Copy link
Author

redacted screen shot of Aspire 8.0.0-preview.4.24156.9 dashboard structured logs with timestamps having only one-second precision

@KalleOlaviNiemitalo
Copy link
Author

KalleOlaviNiemitalo commented Mar 27, 2024

I wonder if the regexp could be just (s|ss) with no separators; that ought to find the longest leftmost match anyway. I guess not; it would incorrectly match s within quotation marks too.

@KalleOlaviNiemitalo
Copy link
Author

"da-DK" doesn't use colons in LongTimePattern either: "HH.mm.ss"

@JamesNK
Copy link
Member

JamesNK commented Mar 27, 2024

Thanks, that's a bug. Fix looks pretty simple: #3215

@drewnoakes
Copy link
Member

We should file an issue with the docs repo too, as this approach came from there.

@KalleOlaviNiemitalo
Copy link
Author

KalleOlaviNiemitalo commented Apr 8, 2024

I filed the docs issue at dotnet/docs#40356.

The French (Canada) culture "fr-CA" has LongTimePattern = HH 'h' mm 'min' ss 's'. The updated regular expression (:ss|\.ss|:s|\.s) from #3215 still won't match anything in that. I guess this should be transformed to HH 'h' mm 'min' ss','fff 's'.

@KalleOlaviNiemitalo
Copy link
Author

PS C:\> [CultureInfo]::GetCultures([CultureTypes]::SpecificCultures) | Group-Object -Property { $_.DateTimeFormat.LongTimePattern }

Count Name                      Group
----- ----                      -----
    1  HH:mm:ss                 {my-MM}
    2 H:m:s                     {yo-BJ, yo-NG}
   23 H:mm:ss                   {ar-IL, ca-AD, ca-ES, ca-ES-VALENCIA…}
    1 H:mm:ss 'ч'.              {bg-BG}
  162 h:mm:ss tt                {aa-DJ, aa-ER, aa-ET, af-NA…}
    3 H.mm.ss                   {en-FI, fi-FI, smn-FI}
    1 HH 'h' mm 'min' ss 's'    {fr-CA}
  344 HH:mm:ss                  {af-ZA, agq-CM, ar-KM, ar-MA…}
    3 hh:mm:ss tt               {gu-IN, kn-IN, sa-IN}
    6 HH.mm.ss                  {da-DK, da-GL, en-DK, id-ID…}
    1 tt 'ga' h:mm:ss           {ee-GH}
    5 tt h:mm:ss                {ko-KP, ko-KR, ta-IN, ta-MY…}
    1 tt h.mm.ss                {as-IN}
    7 tth:mm:ss                 {zh-Hans-CN, zh-Hans-HK, zh-Hans-MO, zh-Hans-SG…}

@KalleOlaviNiemitalo
Copy link
Author

KalleOlaviNiemitalo commented Apr 8, 2024

Apart from the time separator problem, I'd suggest injecting a backslash \ in front of each character of the NumberDecimalSeparator rather than single quotes ' around it, in order to ensure that ToString will parse those characters as literals even if one of them is a single quote character itself.

return MatchSecondsInTimeFormatPattern().Replace(longTimePattern, $"$1'{key.NumberDecimalSeparator}'{millisecondFormat}");

@github-actions github-actions bot locked and limited conversation to collaborators May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants