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

Combine console logs pages #1034

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Combine console logs pages #1034

merged 1 commit into from
Nov 28, 2023

Conversation

tlmii
Copy link
Member

@tlmii tlmii commented Nov 25, 2023

Resolves #176

After #987, merging the logs pages is the next logical step.

  • Removed the Project Logs, Container Logs and Executable Logs pages
  • Created a new Console Logs page
  • Changed the menu to have a Monitoring section that has Console logs, Structured Logs, Traces and Metrics under it and removed the old Logs section that had all of the console logs and structured logs
  • Removed some commented code from the menu that should have been removed earlier

Unlike #987, not much to show from a visuals perspective other than the changes to the menu structure:
image

Note: #176 had discussion of indicating the type of resource that is selected. For the moment, I've left that out of this PR and plan to do follow up once people take a look and we can agree on a design (icon in select? text in the header?).

Copy link
Member

@davidfowl davidfowl left a comment

Choose a reason for hiding this comment

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

I think this is fine as it pretty much matches structured logs.

@JamesNK
Copy link
Member

JamesNK commented Nov 27, 2023

The monitoring pages show resources ordered alphabetically. The resources page orders them by type, then alphabetically. How do we feel about that consistency?

@tlmii
Copy link
Member Author

tlmii commented Nov 27, 2023

The monitoring pages show resources ordered alphabetically. The resources page orders them by type, then alphabetically. How do we feel about that consistency?

I'm conflicted about it (don't like the inconsistency) but it was done on purpose:

For the resources page, with the new columns whose data vary by type, grouping the rows by type helped visual scanning (imo at least) - when the columns with different data in them were intermingled it felt harder to scan them.

For the monitoring pages, without any extra data to identify why they were not alphabetical (in other words no type column or icon) it felt more natural to be alphabetical.

I'll readily admit these are subjective takes and am happy to tweak or take feedback. But that's why I went that way after fiddling with a few variants.

@JamesNK
Copy link
Member

JamesNK commented Nov 27, 2023

When you click on the console logs link in the menu, what decides which app to display?

Structured logs and traces have an "All" option and select it by default. But that doesn't make sense for console logs.
Metrics doesn't display anything by default and prompts you to choose a service (note: service should be renamed to resource)

image

I think the console logs could do the same thing. That page would also be displayed if a bad name is in the url, e.g. http://localhost:15888/ConsoleLogs/ThisDoesntExist

@tlmii
Copy link
Member Author

tlmii commented Nov 28, 2023

When you click on the console logs link in the menu, what decides which app to display?

@JamesNK This PR didn't change the way that worked from before - it selects the first one in the list (which is sorted alphabetically). I think this is a good suggestion though. I filed #1055 to track it.

@tlmii tlmii merged commit b7a7403 into dotnet:main Nov 28, 2023
3 checks passed
@tlmii tlmii deleted the dev/merge-logs branch November 28, 2023 00:28
@github-actions github-actions bot locked and limited conversation to collaborators Apr 28, 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.

Consider merging Project Logs and Container Logs pages
3 participants