Improve log display #3768
Improve log display #3768
Conversation
|
Skipping CI for Draft Pull Request. |
tekton-robot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all |
|
E2E failed on import resources test when selecting the import namespace. Not seeing the same issue locally, rerunning to see if it persists on prow /test pull-tekton-dashboard-integration-tests |
|
Failed in the same place. It's passing consistently locally, both in Electron and a real browser. Will need to debug next week… |
|
/test pull-tekton-dashboard-integration-tests |
|
@AlanGreene: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
| export function getLogLevels() { | ||
| let logLevels = localStorage.getItem('tkn-logs-levels'); | ||
| if (logLevels) { | ||
| logLevels = JSON.parse(logLevels); |
There was a problem hiding this comment.
Worth wrapping in a try-catch and return the default logLevels in case an error is encountered parsing the JSON, if bad data somehow exists in tkn-logs-levels?
|
One thing I noticed at https://alangreene.github.io/dashboard/?path=/story/components-log--toolbar, the dropdown menu disappears every time you change a logLevel option. Would be handy to be keep it up if user wants to add/remove multiple levels |
Yes, that's something we've already discussed with design. Other similar tools behave in this way, including the Google Chrome dev tools. We may revisit based on user feedback and provide a richer popover similar to the Mezmo log level selection or something else entirely. The thinking behind leaving it the way it currently is is that users are unlikely to be changing the selected log levels very often, so the extra clicks aren't overly burdensome. If this proves untrue, we'll explore other approaches. |
|
Forgot to add an update this morning. The Carbon issue causing the E2E failure is carbon-design-system/carbon#18145, which explains why it wasn't reproduced locally in dev mode. |
AlanGreene
force-pushed
the
logs_display_improvements
branch
4 times, most recently
from
4ff63ac
to
08267d4
Compare
Add support for toggling display of timestamps in the logs, so the user no longer has to navigate away to the settings page to do this. This is available on both PipelineRun and TaskRun pages. We always request timestamps and just show / hide them depending on the user's preference. This is persisted to browser localStorage as with the toggle on the settings page. Add support for detecting GitHub Actions workflow command-style log levels in log output. This provides an improved user experience as it allows for filtering logs to hide unwanted noise, e.g. debug logs, by default. We may consider allowing the format to be customised in a future release depending on user feedback. Refactor the styles so `LogFormat` now correctly owns most of the styling of the log content, with `Log` only responsible for additional styling of the container. Refactor use of the `LogsToolbar` component to allow for customisable use by third-party consumers of the Dashboard components. This means they can much more easily take advantage of the new features, such as toggling timestamps and log levels, without having to reimplement the menu and related code themselves. Eliminate redundant use of `split` and `join` calls when processing the logs, improving performance. `LogFormat` now receives an array of log line objects, pre-parsed into the new structure with the `timestamp`, `level` (optional), and `message` fields. Where a multiline log is encountered, the timestamp of the first line is reused for subsequent lines in that log. Fix issue where in some cases a blank line did not reserve vertical space, leading to cramped display of logs. Now each line is guaranteed to occupy a minimum height, ensuring blank lines output in the logs to aid in readability are preserved in the UI. Update `FormattedDate` to add support for displaying seconds, as this is quite important in the log context. Default to `false` for this setting so existing date / timestamps in other parts of the UI are unaffected. The full raw timestamp as received in the logs in displayed in a tooltip on hover. Update unit tests to reflect the new and changed components and behaviours. Update common PipelineRun E2E to exercise the new log toolbar and validate the log content is rendered as expected. Add new stories to cover the new functionality. Update existing stories to demonstrate use of the new functionality in context. Update Carbon: - resolve issue with Plex Mono font Some glyphs weren't included in the Plex version packaged with previous Carbon releases, resulting in broken formatting for some log content, e.g. using box characters to print tables. In `@carbon/react` 1.71.0 the Plex version has been updated, as well as changing how it's consumed. Instead of a single package with all of the font variants, they're now published as separate packages per font family. Add the `$use-per-family-plex` flag to our config to use these new packages. The custom `$font-path` is still required for compatibility with Vite. - resolve issue with duplicate onChange events from MenuItemSelectable - resolve issue with duplicate onChange events when clearing a ComboBox - document the log viewer feature, the new log format, and the existing external logs support Notes: - colours of the log level badges are based on the colours of the Carbon `Tag` component, with their opacity reduced so they're not as intense due to the potentially large number of them that could be displayed in the logs. These all meet minimum colour contrast ratio required for WCAG 2.0 level AA (i.e. > 4.5:1). - the default log level is 'info' if no log level is explicitly provided in the logs, however we only display the badge when the log level is explicitly set. This avoids unnecessary and unwanted noise / clutter in the logs when not using the new log format. - highlight and hover state included to highlight log lines, aiding in consuming the content, especially with longer log lines where the log level badge may not be adjacent to the content being read. A future update to the log viewer will add support for line wrapping but this is out of scope for this particular change.
AlanGreene
force-pushed
the
logs_display_improvements
branch
from
08267d4
to
fccf42d
Compare
Changes
Related to #2306
Resolves #3737
TODO:
Add support for toggling display of timestamps in the logs, so the user no longer has to navigate away to the settings page to do this. This is available on both PipelineRun and TaskRun pages. We always request timestamps and just show / hide them depending on the user's preference. This is persisted to browser localStorage as with the toggle on the settings page.
Add support for detecting GitHub Actions workflow command-style log levels in log output. This provides an improved user experience as it allows for filtering logs to hide unwanted noise, e.g. debug logs, by default. We may consider allowing the format to be customised in a future release depending on user feedback.
Refactor the styles so
LogFormatnow correctly owns most of the styling of the log content, withLogonly responsible for additional styling of the container.Refactor use of the
LogsToolbarcomponent to allow for customisable use by third-party consumers of the Dashboard components. This means they can much more easily take advantage of the new features, such as toggling timestamps and log levels, without having to reimplement the menu and related code themselves.Eliminate redundant use of
splitandjoincalls when processing the logs, improving performance.LogFormatnow receives an array of log line objects, pre-parsed into the new structure with thetimestamp,level(optional), andmessagefields. Where a multiline log is encountered, the timestamp of the first line is reused for subsequent lines in that log.Fix issue where in some cases a blank line did not reserve vertical space, leading to cramped display of logs. Now each line is guaranteed to occupy a minimum height, ensuring blank lines output in the logs to aid in readability are preserved in the UI.
Update
FormattedDateto add support for displaying seconds, as this is quite important in the log context. Default tofalsefor this setting so existing date / timestamps in other parts of the UI are unaffected. The full raw timestamp as received in the logs in displayed in a tooltip on hover.Update unit tests to reflect the new and changed components and behaviours.
Update common PipelineRun E2E to exercise the new log toolbar and validate the log content is rendered as expected.
Add new stories to cover the new functionality. Update existing stories to demonstrate use of the new functionality in context.
Update Carbon:
resolve issue with Plex Mono font
Some glyphs weren't included in the Plex version packaged with previous Carbon releases, resulting in broken formatting for some log content, e.g. using box characters to print tables.
In
@carbon/react1.71.0 the Plex version has been updated, as well as changing how it's consumed. Instead of a single package with all of the font variants, they're now published as separate packages per font family. Add the$use-per-family-plexflag to our config to use these new packages. The custom$font-pathis still required for compatibility with Vite.resolve issue with duplicate onChange events from MenuItemSelectable
resolve issue with duplicate onChange events when clearing a ComboBox
Notes:
Tagcomponent, with their opacity reduced so they're not as intense due to the potentially large number of them that could be displayed in the logs. These all meet minimum colour contrast ratio required for WCAG 2.0 level AA (i.e. > 4.5:1)./kind feature
/kind design
/kind bug
/kind misc
/kind documentation
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes