-
Notifications
You must be signed in to change notification settings - Fork 383
Delimiting tag set headers #5551
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried visualizing the "Microsoft.AspNetCore.Hosting" provider on a .NET web app? That shows an example of metrics with quite a few dimensions. I'm guessing this extra indent is a tradeoff between making the headers clearer when there are fewer dimensions vs. taking more horizontal space for cases with large numbers of dimensions. As long as the high dimension cases still look OK then tab is great. If high dimension cases don't look good any more than we might want to consider other options like smaller spacing or using "-----" to underline column headers instead.
https://learn.microsoft.com/en-us/aspnet/core/log-mon/metrics/metrics?view=aspnetcore-9.0
So in this case we have a table with headers lined up horizontally. It doesn't look truncated with the added indentation. On the other hand, in such a table one would expect the values to be left-aligned, which they are not. If the header name were short this could be an issue. |
I will see if it looks better with an underline. |
![]() ![]() |
Agreed, the underline looks better to me as well. As a small tweak, I think it might help visually separate the columns if you put a space in the underline in between the columns:
|
This PR is intended to indent tag sets such as those under gc.heap.generation by one extra indent. This makes it clearer that the label gc.heap.generation is a header and not a data value.
Before:

After:

closes #4935