Skip to content

Conversation

@JamesNK
Copy link
Member

@JamesNK JamesNK commented Nov 27, 2024

Description

Fixes #5457

Demo:

consolelogs-dates

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?
Microsoft Reviewers: Open in CodeFlow

@JamesNK JamesNK changed the title Option to hide dates on console logs page Option to hide timestamps on console logs page Nov 27, 2024
@samsp-msft
Copy link
Contributor

Cut the date, but keep the time. The dashboard is not typically used to inspect data from long time periods ago, but understanding the delta and frequency of the log messages based on time is important. Showing a long date isn't needed.

@maddymontaquila
Copy link
Member

Cut the date, but keep the time. The dashboard is not typically used to inspect data from long time periods ago, but understanding the delta and frequency of the log messages based on time is important. Showing a long date isn't needed.

Ya agree with @samsp-msft here.

However this is implemented should play nicely with what we decide to do in the future if we start saving dashboard UI state for a local user - ie, resizing/hiding/adding columns, details window pane size

@JamesNK
Copy link
Member Author

JamesNK commented Nov 27, 2024

Today the timestamp matches the ISO 8601 standard. All developers will recognize what it is. I hesitate to change it to just the time, because then it's no longer immediately recognisable as a timestamp.

I also don't want to overload the user with choice. I think the best choices are simple on and off, which means show all (full timestamp) or nothing.

@adamint
Copy link
Member

adamint commented Nov 27, 2024

This looks good to me, but a request if possible would be to add a command to toggle UTC/local time

@maddymontaquila
Copy link
Member

Today the timestamp matches the ISO 8601 standard. All developers will recognize what it is. I hesitate to change it to just the time, because then it's no longer immediately recognisable as a timestamp.

I also don't want to overload the user with choice. I think the best choices are simple on and off, which means show all (full timestamp) or nothing.

This makes sense. I'd keep it on by default then, but add the option to change it. I think most people are going to expect timestamps like a regular logfile

@JamesNK
Copy link
Member Author

JamesNK commented Nov 27, 2024

The experience here isn't a log file though, it's console output. A default console doesn't include a timestamp with every line. e.g. Windows command prompt or Windows terminal don't include a timestamp.

@maddymontaquila
Copy link
Member

Sure - but we call them "console logs" on the dashboard and I feel like they're spiritually logs. I'm willing to bend if you and others (@leslierichardson95 mainly) feel strongly that it's worth changing the default behavior though

@drewnoakes
Copy link
Member

My vote would be to keep timestamps by default. They add a lot of information when looking at logs to understand system behaviour, and if users don't find the option to turn them on then they'll be missing out on that info.

I agree with @samsp-msft that the date could be hidden by default though. I don't think a value like 12:34:56.123 is confusing in a log view. It's pretty obviously a timestamp. The full date could be in a tool tip.

@JamesNK
Copy link
Member Author

JamesNK commented Nov 28, 2024

PR feedback applied. Seems like consensus is to leave timestamp by default.

Further changes to tweak page can be in follow up PRs.

@JamesNK
Copy link
Member Author

JamesNK commented Nov 28, 2024

BTW this PR depends on #6819 being merged first.

Base automatically changed from jamesnk/consolelogs-commands to main November 29, 2024 00:21
@JamesNK JamesNK force-pushed the jamesnk/hide-consolelog-date branch from dc03006 to 7ad28fd Compare November 29, 2024 00:39
@JamesNK JamesNK enabled auto-merge (squash) November 29, 2024 00:46
@JamesNK
Copy link
Member Author

JamesNK commented Nov 29, 2024

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JamesNK JamesNK merged commit 1a5c0be into main Nov 29, 2024
9 checks passed
@JamesNK JamesNK deleted the jamesnk/hide-consolelog-date branch November 29, 2024 03:22
@github-actions github-actions bot locked and limited conversation to collaborators Dec 29, 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.

Add option to control timestamps on dashboard console log page

6 participants