Skip to content

Conversation

@adamint
Copy link
Member

@adamint adamint commented Mar 26, 2025

Description

Adds the ability to sort by timestamp for traces and structured logs. For this, I had to extend OtlpHelpers.GetItems to also accept a sorting function as is provided to us in GetData in the data grids.

image

Fixes #4190

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?

@adamint adamint requested review from JamesNK and Copilot March 26, 2025 22:31
@adamint adamint self-assigned this Mar 26, 2025
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 sorting functionality by timestamp for both traces and structured logs. Key changes include new tests for sorting, updates to API context objects and helper methods to accept a sort function, and UI component adjustments to enable sortable columns.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/Aspire.Dashboard.Tests/TelemetryRepositoryTests/TraceTests.cs Added tests to validate trace sorting and subset selection
tests/Aspire.Dashboard.Tests/TelemetryRepositoryTests/LogTests.cs Added tests to validate log sorting and subset selection
src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor.cs Updated log page to use sortable columns with a new GridSort instance
src/Aspire.Dashboard/Otlp/Storage/GetLogsContext.cs Extended context with a new SortFunction property
src/Aspire.Dashboard/Otlp/Model/OtlpHelpers.cs Modified GetItems overloads to support sorting
src/Aspire.Dashboard/Components/Pages/Traces.razor.cs Updated traces page to use sortable columns with a new GridSort instance
src/Aspire.Dashboard/Model/TracesViewModel.cs Updated GetTraces method to accept a sort function
src/Aspire.Dashboard/Model/StructuredLogsViewModel.cs Updated GetLogs method to accept a sort function
src/Aspire.Dashboard/Otlp/Storage/TelemetryRepository.cs Enhanced GetLogs/GetTraces to propagate sort functions; issues with parameter order in GetLogs call
src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor Enabled sorting on the timestamp column
src/Aspire.Dashboard/Components/Pages/Traces.razor Enabled sorting on the timestamp column

@JamesNK
Copy link
Member

JamesNK commented Mar 27, 2025

A problem with sorting in this direction:

If you're not scrolled to bottom, new logs or traces will push what you're currently viewing off the page. Ideally the grid would keep position. In other words, as new items were added to the grid, the scrollbar would update to keep the currently viewed items on the screen.

Example of the problem:

logs-order-scrolling

Pausing collection is a work around. But it's not a good experience without pausing.

@adamint
Copy link
Member Author

adamint commented Mar 28, 2025

A problem with sorting in this direction:

If you're not scrolled to bottom, new logs or traces will push what you're currently viewing off the page. Ideally the grid would keep position. In other words, as new items were added to the grid, the scrollbar would update to keep the currently viewed items on the screen.

Example of the problem:

logs-order-scrolling logs-order-scrolling

Pausing collection is a work around. But it's not a good experience without pausing.

So, dependent on #8184?

@adamint
Copy link
Member Author

adamint commented Mar 29, 2025

Or do we also stop the page from scrolling

@JamesNK
Copy link
Member

JamesNK commented Mar 29, 2025

Investigate what is involved to keep position. This must be a common problem with examples of solutions.

I don't think this needs to be in 9.2.

@adamint adamint marked this pull request as draft April 9, 2025 19:49
@adamint
Copy link
Member Author

adamint commented Apr 9, 2025

Reached out to Vincent for advice.

@vnbaaij
Copy link
Contributor

vnbaaij commented Apr 9, 2025

We don't have any good ways of maintaining or changing scroll positions in the DataGrid, I'm afraid...

@danmoseley
Copy link
Member

closing stale PR's. this seems like it needs more design work if we do it.

@danmoseley danmoseley closed this Apr 30, 2025
@github-actions github-actions bot locked and limited conversation to collaborators May 31, 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.

Add configurable timestamp sorting

4 participants