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

[APM] Fix occurrences cut off value to be fully visible #175307

Conversation

jennypavlova
Copy link
Contributor

@jennypavlova jennypavlova commented Jan 23, 2024

Closes 174021

Summary

This PR fixes the occurrences column cut-off value and makes it fully visible.
My first idea was to try to make a custom styling for the specific Occurrences column but after that, I found that inside the SparkPlot component I could make the label visible for all the similar cases where we show label and chart in a table by removing the grow={false} and whiteSpace: 'nowrap' style.

Before After
image image

With the change, it is also visible if the window is resized ( both smaller and bigger):

apm_error_table_resize.mov

Testing

  1. Use synthtrace to generate data
    1. One way is to change the failedTimestamps inside the simple_trace to something bigger (add several 0s but not toooo big as it may result in an error)
    2. If you don't want to do that the value can be adjusted in the browser like in the resizing example video and simple_trace can remain the same
    3. After that run node scripts/synthtrace simple_trace.ts
  2. Open a service and check the Overview and Errors tabs (as well as the other table values using label and chart)

@jennypavlova jennypavlova self-assigned this Jan 23, 2024
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@jennypavlova jennypavlova added release_note:fix apm:service-overview Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Jan 23, 2024
@jennypavlova
Copy link
Contributor Author

/ci

@jennypavlova jennypavlova marked this pull request as ready for review January 23, 2024 14:16
@jennypavlova jennypavlova requested a review from a team as a code owner January 23, 2024 14:16
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.1MB 3.1MB -33.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jennypavlova

Copy link
Contributor

@crespocarlos crespocarlos left a comment

Choose a reason for hiding this comment

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

LGTM!

<EuiFlexItem grow={false} style={{ whiteSpace: 'nowrap' }}>
{valueLabel}
</EuiFlexItem>
<EuiFlexItem>{valueLabel}</EuiFlexItem>
Copy link
Contributor

Choose a reason for hiding this comment

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

Another option could be shortening big numbers like 1000 > 1k, 1000000 > 1m, and etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea, my concern is that we are losing precision and for such cases, I think it might be important to know the exact number - also this will work for different labels not only numbers.

@jennypavlova jennypavlova merged commit 6ca8cfb into elastic:main Jan 24, 2024
20 checks passed
@jennypavlova jennypavlova deleted the 174021-apm-errors-overview-table-occurrences-value-fix branch January 24, 2024 10:31
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 24, 2024
Copy link
Contributor

@kpatticha kpatticha left a comment

Choose a reason for hiding this comment

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

Thanks for improving this one @jennypavlova . We recently had a new SDH about this so I'm sure the user will be happy with the fix :D

However, I've noticed that the fix is not applied on the error details page.

image

it would be great if we could improve this page as well.

lcawl pushed a commit to lcawl/kibana that referenced this pull request Jan 26, 2024
Closes [174021](elastic#174021)
## Summary

This PR fixes the occurrences column cut-off value and makes it fully
visible.
My first idea was to try to make a custom styling for the specific
`Occurrences` column but after that, I found that inside the `SparkPlot`
component I could make the label visible for all the similar cases where
we show label and chart in a table by removing the `grow={false}` and
`whiteSpace: 'nowrap'` style.

| Before  | After |
| ------- | ----- |
|
![image](https://github.com/elastic/kibana/assets/14139027/a1bf7bd8-0828-437f-9b91-42e59b587012)
|
![image](https://github.com/elastic/kibana/assets/14139027/71df43f2-01a4-40f7-95ea-506741b3b1ce)
|

With the change, it is also visible if the window is resized ( both
smaller and bigger):



https://github.com/elastic/kibana/assets/14139027/d5f3f501-b726-4e01-aea1-a0c890a2e0ee


## Testing
1. Use synthtrace to generate data 
1. One way is to change the `failedTimestamps` inside the
[simple_trace](https://github.com/elastic/kibana/blob/f7bd91763f7b87e9f3528cee0bff2e94b192eef8/packages/kbn-apm-synthtrace/src/scenarios/simple_trace.ts#L24)
to something bigger (add several `0`s but not toooo big as it may result
in an error)
2. If you don't want to do that the value can be adjusted in the browser
like in the resizing example video and `simple_trace` can remain the
same
    3. After that run `node scripts/synthtrace simple_trace.ts`
2. Open a service and check the Overview and Errors tabs (as well as the
other table values using label and chart)
@jennypavlova
Copy link
Contributor Author

However, I've noticed that the fix is not applied on the error details page.

Hey @kpatticha, thanks for the comment. I missed it, sorry for the delay. I am not able to reproduce this also on a smaller screen size (locally using the latest main):
image
image
image

Are there any specific services I should check in order to see it? Can you please check if you can reproduce it again on the latest main and I can add an issue to fix this.

CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
Closes [174021](elastic#174021)
## Summary

This PR fixes the occurrences column cut-off value and makes it fully
visible.
My first idea was to try to make a custom styling for the specific
`Occurrences` column but after that, I found that inside the `SparkPlot`
component I could make the label visible for all the similar cases where
we show label and chart in a table by removing the `grow={false}` and
`whiteSpace: 'nowrap'` style.

| Before  | After |
| ------- | ----- |
|
![image](https://github.com/elastic/kibana/assets/14139027/a1bf7bd8-0828-437f-9b91-42e59b587012)
|
![image](https://github.com/elastic/kibana/assets/14139027/71df43f2-01a4-40f7-95ea-506741b3b1ce)
|

With the change, it is also visible if the window is resized ( both
smaller and bigger):



https://github.com/elastic/kibana/assets/14139027/d5f3f501-b726-4e01-aea1-a0c890a2e0ee


## Testing
1. Use synthtrace to generate data 
1. One way is to change the `failedTimestamps` inside the
[simple_trace](https://github.com/elastic/kibana/blob/f7bd91763f7b87e9f3528cee0bff2e94b192eef8/packages/kbn-apm-synthtrace/src/scenarios/simple_trace.ts#L24)
to something bigger (add several `0`s but not toooo big as it may result
in an error)
2. If you don't want to do that the value can be adjusted in the browser
like in the resizing example video and `simple_trace` can remain the
same
    3. After that run `node scripts/synthtrace simple_trace.ts`
2. Open a service and check the Overview and Errors tabs (as well as the
other table values using label and chart)
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
Closes [174021](elastic#174021)
## Summary

This PR fixes the occurrences column cut-off value and makes it fully
visible.
My first idea was to try to make a custom styling for the specific
`Occurrences` column but after that, I found that inside the `SparkPlot`
component I could make the label visible for all the similar cases where
we show label and chart in a table by removing the `grow={false}` and
`whiteSpace: 'nowrap'` style.

| Before  | After |
| ------- | ----- |
|
![image](https://github.com/elastic/kibana/assets/14139027/a1bf7bd8-0828-437f-9b91-42e59b587012)
|
![image](https://github.com/elastic/kibana/assets/14139027/71df43f2-01a4-40f7-95ea-506741b3b1ce)
|

With the change, it is also visible if the window is resized ( both
smaller and bigger):



https://github.com/elastic/kibana/assets/14139027/d5f3f501-b726-4e01-aea1-a0c890a2e0ee


## Testing
1. Use synthtrace to generate data 
1. One way is to change the `failedTimestamps` inside the
[simple_trace](https://github.com/elastic/kibana/blob/f7bd91763f7b87e9f3528cee0bff2e94b192eef8/packages/kbn-apm-synthtrace/src/scenarios/simple_trace.ts#L24)
to something bigger (add several `0`s but not toooo big as it may result
in an error)
2. If you don't want to do that the value can be adjusted in the browser
like in the resizing example video and `simple_trace` can remain the
same
    3. After that run `node scripts/synthtrace simple_trace.ts`
2. Open a service and check the Overview and Errors tabs (as well as the
other table values using label and chart)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:service-overview backport:skip This commit does not require backporting release_note:fix Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Obs UX] APM - Services - Errors Overview table - Occurrences value is cut off
7 participants