Skip to content

Conversation

@JamesNK
Copy link
Member

@JamesNK JamesNK commented Jul 8, 2025

Description

Add log entries to trace details. Visual buttons for each log entry is displayed on associated spans. Can hover for a tooltip or click to open log entry details.

trace-details-logs

Future improvements (probably in a future PR)

  • Add span events in the same way
  • Give error log entries a different color or shape

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@Copilot Copilot AI review requested due to automatic review settings July 8, 2025 03:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds structured log entries to the trace detail view by extending the span waterfall view models, adding buttons for each log on spans, and wiring up tooltips and detail panels.

  • Updated SpanWaterfallViewModel.Create to accept a list of OtlpLogEntry and populate a new SpanLogs property.
  • Introduced StructureLogsDetailsViewModel.GetEventName for consistent event-name resolution.
  • Enhanced the TraceDetail page and CSS to render clickable log-entry buttons with tooltips and detail panels.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/Aspire.Dashboard.Tests/Model/SpanWaterfallViewModelTests.cs Adjusted Create calls to include the new logs argument.
src/Aspire.Dashboard/Model/StructureLogsDetailsViewModel.cs Added GetEventName helper for log entry event names.
src/Aspire.Dashboard/Model/Otlp/SpanWaterfallViewModel.cs Extended Create to take logs and build SpanLogs.
src/Aspire.Dashboard/Components/Pages/TraceDetail.razor.css Added styles for log-entry buttons and tooltips.
src/Aspire.Dashboard/Components/Pages/TraceDetail.razor.cs Injected localizers, managed SelectedData, and toggle logic.
src/Aspire.Dashboard/Components/Pages/TraceDetail.razor Rendered log-entry buttons, tooltips, and detail views.
src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor Refactored event-name logic to use GetEventName.
Comments suppressed due to low confidence (3)

tests/Aspire.Dashboard.Tests/Model/SpanWaterfallViewModelTests.cs:29

  • No tests cover the new log-related functionality in SpanWaterfallViewModel.Create. Add unit tests that supply non-empty logs input and verify that the SpanLogs property of the resulting view models is populated correctly.
        var vm = SpanWaterfallViewModel.Create(trace, [], new SpanWaterfallViewModel.TraceDetailState([], []));

src/Aspire.Dashboard/Model/StructureLogsDetailsViewModel.cs:13

  • The public method GetEventName lacks XML documentation comments. Please add triple-slash summaries and <remarks> to explain its purpose and fallback behavior.
    public static string GetEventName(OtlpLogEntry logEntry, IStringLocalizer<Dashboard.Resources.StructuredLogs> loc)

src/Aspire.Dashboard/Components/Pages/TraceDetail.razor.css:242

  • The selector ::deep.log-tooltip-title-container is missing a space between ::deep and the class, which likely prevents it from applying. It should be ::deep .log-tooltip-title-container.
::deep.log-tooltip-title-container {

var eventName = StructureLogsDetailsViewModel.GetEventName(item.LogEntry, StructuredLogsLoc);
var isSelected = SelectedData?.LogEntryViewModel?.LogEntry.InternalId == item.LogEntry.InternalId;

<button id="@buttonId"
Copy link

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log entry buttons lack a visible label or aria-label, which makes them inaccessible to screen readers. Add an aria-label attribute (for example, aria-label="<event name>") to each button to provide an accessible name.

Copilot uses AI. Check for mistakes.

@davidfowl
Copy link
Member

Did you stress test this?

@JamesNK
Copy link
Member Author

JamesNK commented Jul 9, 2025

Did you stress test this?

I tested with 10,000 logs in one trace. Many HTML tooltips can be a performance hit so I limited to 500. The fallback is browser tooltips.

There is still a 1 second pause to render the page, but that seems better than hiding some buttons. And realistically a trace with 10,000 logs is a rare event.

@JamesNK JamesNK enabled auto-merge (squash) July 10, 2025 01:00
@JamesNK JamesNK merged commit 9fe8413 into main Jul 10, 2025
252 checks passed
@JamesNK JamesNK deleted the jamesnk/trace-details-logs branch July 10, 2025 01:23
@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants