Skip to content

Persist state in structured logs page#2161

Merged
adamint merged 13 commits intomicrosoft:mainfrom
adamint:dev/adamint/make-structured-logs-stateful
Feb 16, 2024
Merged

Persist state in structured logs page#2161
adamint merged 13 commits intomicrosoft:mainfrom
adamint:dev/adamint/make-structured-logs-stateful

Conversation

@adamint
Copy link
Copy Markdown
Member

@adamint adamint commented Feb 9, 2024

Fixes #1694 by persisting all state in the structured logs page.

Level/application are stored as usual (just strings), while log filters are serialized to/deserialized from the following format:

fieldName:condition:value

where value is url-encoded

Animation

Microsoft Reviewers: Open in CodeFlow

@ghost ghost added the area-dashboard label Feb 9, 2024

if (serializable.Filters.Count > 0)
{
queryParameters.Add("filters", JsonSerializer.Serialize(serializable.Filters));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Serializing to JSON isn't the right approach here in my opinion. A URL is intended to be human readable. JSON + URL escaping obfuscates what is happening too much.

(btw I also don't like the service instance ID GUID that is added at the moment but haven't found a perfect way to replace it yet)

Maybe we could use GitHub's search format here: https://docs.github.com/en/issues/planning-and-tracking-with-projects/customizing-views-in-your-project/filtering-projects

Message == "test" = message:test
Message == "hello world" = message:"hello world" (double quote when there are spaces)
Message CONTAINS "hello world" = message:*"hello world"* (add stars for contains)
Message != "hello world" = -message:"hello world" (add minus for negation)
Message NOT CONTAINS "hello world" = -message:*"hello world"*

Example:
https://github.com/dotnet/aspire/issues?q=is%3Aopen+is%3Aissue+-author%3A%40me+sort%3Aupdated-desc+

You would be writing a new serializer/deserializer so it would be much more work. Maybe it's a future improvement?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm happy to do either. What do you mean by not liking the service instance id guid?

Copy link
Copy Markdown
Member

@JamesNK JamesNK Feb 14, 2024

Choose a reason for hiding this comment

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

If you think parsing/writing filters won't take too long then ago ahead.

Service instance id guid is this:

image

Console logs does it better:

image

We might be able to use the resource ID as the service instance ID now that it is unique per run.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For now, I made a simple filter parser

Similar to github, + separates individual filters, and to avoid confusion, there's a second parameter (separated by a colon) signifying the condition. The filter value is url encoded

ie:

filters=Message:contains:app+Application:!equals:frontend

Copy link
Copy Markdown
Member Author

@adamint adamint Feb 15, 2024

Choose a reason for hiding this comment

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

re: your comment about service instance ID, I see that's used in other pages too. I'll change all of them in a separate PR and request your review

edit: I opened a separate PR for the other pages, but did structured logs page in this PR
#2255

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have we accounted for when : is present in the string being searched? This is common in log strings where message:*"myApp: Starting up..." maybe searched for

Copy link
Copy Markdown
Member Author

@adamint adamint Feb 16, 2024

Choose a reason for hiding this comment

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

Yes, colon is url-encoded!

@adamint adamint requested a review from JamesNK February 15, 2024 18:08
Adam Ratzman added 3 commits February 16, 2024 10:34
# Conflicts:
#	src/Aspire.Dashboard/Components/Pages/ConsoleLogs.razor
#	src/Aspire.Dashboard/Components/Pages/ConsoleLogs.razor.cs
#	src/Aspire.Dashboard/Components/Pages/Metrics.razor
#	src/Aspire.Dashboard/Components/Pages/Metrics.razor.cs
#	src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor
#	src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor.cs
@adamint adamint enabled auto-merge (squash) February 16, 2024 21:08
@adamint adamint merged commit 4c3a22a into microsoft:main Feb 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 22, 2024
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.

[WebToolsE2E][Aspire] On the dashboard, the filter doesn't work after clicking 'Structured logs' again.

4 participants