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

fix(console): remove histogram minimum count #424

Merged
merged 1 commit into from
May 17, 2023
Merged

fix(console): remove histogram minimum count #424

merged 1 commit into from
May 17, 2023

Conversation

hds
Copy link
Collaborator

@hds hds commented May 17, 2023

The sparklines histogram used to visualize poll times and scheduled
times renders a legend for both axes. On the horizontal axis, it
displays the durations for the minimum and maximum buckets. On the
vertical axis, it displays the bucket count minimum and maximums.

Unfortunately, the minimum values for each axis are places together,
which can be confusing:

╭Poll Times Histogram─────────────────────────────────╮
│46█                                                  │
│  █                                                  │
│  █                                                  │
│  █                                                  │
│  █ ▂                                                │
│  █▁█▇▃▁▁           ▁     ▁▁                       ▁ │
│ 031.23µs                                      7.11ms│
╰─────────────────────────────────────────────────────╯
  ^|----|
  │  ╰─ This is the minimum bucket duration value
  ╰── This is the minimum count

This can be confusing because it looks like there is an erroneous
leading zero on the minimum bucket duration.

If the minimum value is not zero, then the vertical axis minimum is
updated, but the bottom of the sparkline still represents zero. This
sitaution is unlikely, I had to add a fixed value to the bucket counts
to generate the histogram below.

╭Poll Times Histogram─────────────────────────────────╮
│32██                                                 │
│  ██▇    ▁               ▁   ▁  ▁ ▁                  │
│  ███▇▆▆▆█▇▆▇▆▆▆▆▆▆▆▇▆▆▆▆█▆▇▆█▆▇█▆█▇▇▇▆▇▇▆▇▆▆▇▆▆▆▆▆▇ │
│  ██████████████████████████████████████████████████ │
│  ██████████████████████████████████████████████████ │
│  ██████████████████████████████████████████████████ │
│2033.79µs                                   536.58µs │
╰─────────────────────────────────────────────────────╯
 ^^|----|
  │  ╰─ This is the minimum bucket duration value
  ╰── This is the minimum count

This is probably misleading.

This change removes the minimum label on the vertical axis entirely.
This removes the confusion around the "erroneous" leading zero and since
the bottom of the graph is always zero anyway, it should be clear to
users without having to add the label.

The resulting histogram looks like:

╭Poll Times Histogram─────────────────────────────────╮
│27 █▄                                                │
│   ██                                                │
│   ██                                                │
│   ██                                                │
│   ██▂                                               │
│  ▃███▃▁ ▁      ▁▃ ▃▃▃▁▃▅▁▃ ▃▃▃▃▃▁▁▁   ▁  ▁         ▁│
│  28.93µs                                   811.01µs │
╰─────────────────────────────────────────────────────╯

@hds hds requested a review from a team as a code owner May 17, 2023 15:38
The sparklines histogram used to visualize poll times and scheduled
times renders a legend for both axes. On the horizontal axis, it
displays the durations for the minimum and maximum buckets. On the
vertical axis, it displays the bucket count minimum and maximums.

Unfortunately, the minimum values for each axis are places together,
which can be confusing:

```text
╭Poll Times Histogram─────────────────────────────────╮
│46█                                                  │
│  █                                                  │
│  █                                                  │
│  █                                                  │
│  █ ▂                                                │
│  █▁█▇▃▁▁           ▁     ▁▁                       ▁ │
│ 031.23µs                                      7.11ms│
╰─────────────────────────────────────────────────────╯
  ^|----|
  │  ╰─ This is the minimum bucket duration value
  ╰── This is the minimum count
```

This can be confusing because it looks like there is an erroneous
leading zero on the minimum bucket duration.

If the minimum value is not zero, then the vertical axis minimum is
updated, but the bottom of the sparkline still represents zero. This
sitaution is unlikely, I had to add a fixed value to the bucket counts
to generate the histogram below.

```text
╭Poll Times Histogram─────────────────────────────────╮
│32██                                                 │
│  ██▇    ▁               ▁   ▁  ▁ ▁                  │
│  ███▇▆▆▆█▇▆▇▆▆▆▆▆▆▆▇▆▆▆▆█▆▇▆█▆▇█▆█▇▇▇▆▇▇▆▇▆▆▇▆▆▆▆▆▇ │
│  ██████████████████████████████████████████████████ │
│  ██████████████████████████████████████████████████ │
│  ██████████████████████████████████████████████████ │
│2033.79µs                                   536.58µs │
╰─────────────────────────────────────────────────────╯
 ^^|----|
  │  ╰─ This is the minimum bucket duration value
  ╰── This is the minimum count
```

This is probably misleading.

This change removes the minimum label on the vertical axis entirely.
This removes the confusion around the "erroneous" leading zero and since
the bottom of the graph is always zero anyway, it should be clear to
users without having to add the label.

The resulting histogram looks like:

```text
╭Poll Times Histogram─────────────────────────────────╮
│27 █▄                                                │
│   ██                                                │
│   ██                                                │
│   ██                                                │
│   ██▂                                               │
│  ▃███▃▁ ▁      ▁▃ ▃▃▃▁▃▅▁▃ ▃▃▃▃▃▁▁▁   ▁  ▁         ▁│
│  28.93µs                                   811.01µs │
╰─────────────────────────────────────────────────────╯
```
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Thanks, this definitely looks less confusing!

Another potential solution, if we did want to display the minimum value, would be to move the axis labels outside of the bounding box we draw around the histogram, so that it looks like this:

  ╭Poll Times Histogram───────────────────────────────╮
46│█                                                  │
  │█                                                  │
  │█                                                  │
  │█ ▂                                                │
  │█▁█▇▃▁▁           ▁     ▁▁                       ▁ │
 0╰───────────────────────────────────────────────────╯
   31.23µs                                          7.11ms

I think this would probably be substantially more work, though, and the current solution already solves a lot of the potential ambiguity, so I'm happy to merge this as-is and if we want to sink more time into making it fancier, we can do that later!

@hawkw hawkw merged commit 2617504 into tokio-rs:main May 17, 2023
@hds
Copy link
Collaborator Author

hds commented May 17, 2023

I think that experimenting with where we place the labels would be interesting. We'd have to make sure that they don't end up confusing things with the other widgets' bounding boxes.

hawkw pushed a commit that referenced this pull request Sep 29, 2023
The sparklines histogram used to visualize poll times and scheduled
times renders a legend for both axes. On the horizontal axis, it
displays the durations for the minimum and maximum buckets. On the
vertical axis, it displays the bucket count minimum and maximums.

Unfortunately, the minimum values for each axis are places together,
which can be confusing:

```text
╭Poll Times Histogram─────────────────────────────────╮
│46█                                                  │
│  █                                                  │
│  █                                                  │
│  █                                                  │
│  █ ▂                                                │
│  █▁█▇▃▁▁           ▁     ▁▁                      ▁ │
│ 031.23µs                                      7.11ms│
╰─────────────────────────────────────────────────────╯
  ^|----|
  │  ╰─ This is the minimum bucket duration value
  ╰── This is the minimum count
```

This can be confusing because it looks like there is an erroneous
leading zero on the minimum bucket duration.

If the minimum value is not zero, then the vertical axis minimum is
updated, but the bottom of the sparkline still represents zero. This
sitaution is unlikely, I had to add a fixed value to the bucket counts
to generate the histogram below.

```text
╭Poll Times Histogram─────────────────────────────────╮
│32██                                                 │
│  ██▇    ▁               ▁   ▁  ▁ ▁                  │
│  ███▇▆▆▆█▇▆▇▆▆▆▆▆▆▆▇▆▆▆▆█▆▇▆█▆▇█▆█▇▇▇▆▇▇▆▇▆▆▇▆▆▆▆▆▇ │
│  ██████████████████████████████████████████████████ │
│  ██████████████████████████████████████████████████ │
│  ██████████████████████████████████████████████████ │
│2033.79µs                                   536.58µs │
╰─────────────────────────────────────────────────────╯
 ^^|----|
  │  ╰─ This is the minimum bucket duration value
  ╰── This is the minimum count
```

This is probably misleading.

This change removes the minimum label on the vertical axis entirely.
This removes the confusion around the "erroneous" leading zero and since
the bottom of the graph is always zero anyway, it should be clear to
users without having to add the label.

The resulting histogram looks like:

```text
╭Poll Times Histogram─────────────────────────────────╮
│27 █▄                                                │
│   ██                                                │
│   ██                                                │
│   ██                                                │
│   ██▂                                               │
│  ▃███▃▁ ▁      ▁▃ ▃▃▃▁▃▅▁▃ ▃▃▃▃▃▁▁▁   ▁  ▁         ▁│
│  28.93µs                                   811.01µs │
╰─────────────────────────────────────────────────────╯
```
hawkw added a commit that referenced this pull request Sep 29, 2023
# Changelog

All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## tokio-console-v0.1.10 - (2023-09-29)

[7d009f8](https://github.com/tokio-rs/console/commit/7d009f87120ce0c89f5f9c5311f05b6756ca770f)...[7d009f8](https://github.com/tokio-rs/console/commit/7d009f87120ce0c89f5f9c5311f05b6756ca770f)

### <a id = "tokio-console-v0.1.10-breaking"></a>Breaking Changes
- **Update Tonic and Prost dependencies ([#364](#364 ([f9b8e03](https://github.com/tokio-rs/console/commit/f9b8e03bd7ee1d0edb441c94a93a350d5b06ed3b))<br />This commit updates the public dependencies `prost` and `tonic` to
semver-incompatible versions (v0.11.0 and v0.8.0, respectively). This is
a breaking change for users who are integrating the `console-api` protos
with their own `tonic` servers or clients.
- **Update `tonic` to v0.10 and increase MSRV to 1.64 ([#464](#464 ([96e62c8](https://github.com/tokio-rs/console/commit/96e62c83ef959569bb062dc8fee98fa2b2461e8d))<br />This is a breaking change for users of `console-api` and
`console-subscriber`, as it changes the public `tonic` dependency to a
semver-incompatible version. This breaks compatibility with `tonic`
0.9.x and `prost` 0.11.x.

### Added

- [**breaking**](#tokio-console-v0.1.10-breaking) Update Tonic and Prost dependencies ([#364](#364)) ([f9b8e03](f9b8e03))
- Only suggest opening issues for panics ([#365](#365)) ([da2a89c](da2a89c))
- Init error handling before subcmds ([#365](#365)) ([ec66eda](ec66eda))
- Filter out boring frames in backtraces ([#365](#365)) ([95a5e54](95a5e54))
- Include config options in autogenerated issues ([#365](#365)) ([3244a1f](3244a1f))
- Reduce decimal digits in UI ([#402](#402)) ([c13085e](c13085e))
- Use tokio task ids in task views ([#403](#403)) ([f5b06d2](f5b06d2))
- Add support for Unix domain sockets ([#388](#388)) ([a944dbc](a944dbc), closes [#296](#296))
- Add scheduled time per task ([#406](#406)) ([f280df9](f280df9))
- Add task scheduled times histogram ([#409](#409)) ([d92a399](d92a399))
- Update `tonic` to 0.9 ([#420](#420)) ([48af1ee](48af1ee))
- Update MSRV to Rust 1.60.0 ([b18ee47](b18ee47))
- Migrate to `ratatui` and update `crossterm` ([#425](#425)) ([b209dd6](b209dd6))
- Help view modal ([#432](#432)) ([359a4e7](359a4e7))
- Add way to inspect details of task from resource view ([#449](#449)) ([132ed4e](132ed4e), closes [#448](#448))
- Add warning for tasks that never yield ([#439](#439)) ([d05fa9e](d05fa9e))
- [**breaking**](#tokio-console-v0.1.10-breaking) Update `tonic` to v0.10 and increase MSRV to 1.64 ([#464](#464)) ([96e62c8](96e62c8))

### Documented

- Update screenshots in README ([#419](#419)) ([e9bcd67](e9bcd67))
- Revert "update screenshots in README ([#419](#419))" ([993a3d9](993a3d9))
- Update screenshots in README ([#421](#421)) ([8a27f96](8a27f96))
- Add column descriptions for all tables ([#431](#431)) ([e3cf82b](e3cf82b))
- Update MSRV version docs to 1.64 ([#467](#467)) ([94a5a51](94a5a51))

### Fixed

- Fix ascii-only flipped input ([#377](#377)) ([652ac34](652ac34))
- Declare `tokio-console` bin as `default-run` ([#379](#379)) ([9ce60ec](9ce60ec))
- Make `retain_for` default to 6s if not specfied ([#383](#383)) ([0a6012b](0a6012b), fixes [#382](#382))
- Enable view-switching keystrokes on details views ([#387](#387)) ([f417d7a](f417d7a))
- Fix `ViewOptions` default lang' ([#394](#394)) ([a1cf1b8](a1cf1b8), fixes [#393](#393))
- Remove `tracing-subscriber` 0.2 from dependencies ([#404](#404)) ([768534a](768534a))
- Fix calculation of busy time during poll ([#405](#405)) ([e2c536a](e2c536a))
- Remove histogram minimum count ([#424](#424)) ([02cf8a6](02cf8a6))
- Remove trailing space from task/resource location ([#443](#443)) ([90e5918](90e5918))
- Make long locations readable ([#441](#441)) ([9428d7f](9428d7f), closes [#411](#411))
- Fix task detail view Id to display remote tokio::task::Id ([#455](#455)) ([70c3952](70c3952))

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants