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.

Sign up for GitHub

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

Jump to bottom

[APM] FIX trace waterfall loading logic to handle empty scenarios #163397

Merged
merged 7 commits into from Aug 9, 2023
Merged

[APM] FIX trace waterfall loading logic to handle empty scenarios #163397

merged 7 commits into from Aug 9, 2023

Conversation

achyutjhunjhunwala
Copy link
Contributor

@achyutjhunjhunwala achyutjhunjhunwala commented Aug 8, 2023 •
edited

Summary

Closes #154705

List of Issues

  • Trace Waterfall not clearing and persisting old data

Trace Explorer Not clearing

  • Infinite Loading of Trace Waterfall where no traces are present and page is reloaded

Infinite Loading

  • After Fixing these 2 issues, found issue with Container Height

Container Height Issue

All issues fixed

All issues resolved

@achyutjhunjhunwala achyutjhunjhunwala added release_note:fix Team:APM All issues that need APM UI Team support labels Aug 8, 2023
@achyutjhunjhunwala achyutjhunjhunwala self-assigned this Aug 8, 2023
@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!)

@achyutjhunjhunwala achyutjhunjhunwala marked this pull request as ready for review August 8, 2023 13:42
@achyutjhunjhunwala achyutjhunjhunwala requested a review from a team as a code owner August 8, 2023 13:42
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@yngrdyn
Copy link
Contributor

yngrdyn commented Aug 9, 2023

Great work @achyutjhunjhunwala!

I find a bit weird having those two messages No data to display and The selected trace cannot be found or maybe it's just the positioning but it's a super nit.

yngrdyn
yngrdyn approved these changes Aug 9, 2023
View reviewed changes
Copy link
Contributor

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

LGTM

...plugins/apm/ftr_e2e/cypress/e2e/read_only_user/transaction_details/transaction_details.cy.ts
@@ -151,4 +151,30 @@ describe('Transaction details', () => {
});
});
});

describe('when changing filters which results in no trace samples', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@achyutjhunjhunwala
Copy link
Contributor Author

achyutjhunjhunwala commented Aug 9, 2023 •
edited

Great work @achyutjhunjhunwala!

I find a bit weird having those two messages No data to display and The selected trace cannot be found or maybe it's just the positioning but it's a super nit.

@yngrdyn It's actually an existing message.
No data to display is displayed by the Latency Distribution Chart and The selected trace cannot be found is displayed by Trace Waterfall component. It can be well positioned for sure, but out of scope of this PR

@achyutjhunjhunwala achyutjhunjhunwala enabled auto-merge (squash) August 9, 2023 09:16
@achyutjhunjhunwala achyutjhunjhunwala enabled auto-merge (squash) August 9, 2023 14:01
@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.7MB 3.7MB +80.0B

History

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

cc @achyutjhunjhunwala

@achyutjhunjhunwala achyutjhunjhunwala merged commit fb6c9f0 into elastic:main Aug 9, 2023
21 checks passed
@kibanamachine kibanamachine added v8.10.0 backport:skip This commit does not require backporting labels Aug 9, 2023
@achyutjhunjhunwala achyutjhunjhunwala deleted the Trace-waterfall-is-not-cleared-when-there-are-no-results branch August 9, 2023 17:33
bryce-b pushed a commit to bryce-b/kibana that referenced this pull request Aug 9, 2023
@achyutjhunjhunwala @bryce-b
[APM] FIX trace waterfall loading logic to handle empty scenarios (el…
1393015 
…astic#163397)

## Summary

Closes elastic#154705

## List of Issues

- Trace Waterfall not clearing and persisting old data

![Trace Explorer Not
clearing](https://github.com/elastic/kibana/assets/7416358/5302c5ed-0c92-44ab-81a5-115e32dc8ace)

- Infinite Loading of Trace Waterfall where no traces are present and
page is reloaded

![Infinite
Loading](https://github.com/elastic/kibana/assets/7416358/c5ae1cda-099e-4968-96e5-ad85db38d83a)

- After Fixing these 2 issues, found issue with Container Height

![Container Height
Issue](https://github.com/elastic/kibana/assets/7416358/7fc7ef1b-68dc-4f52-bc2f-c346cfd5f67e)

## All issues fixed

![All issues
resolved](https://github.com/elastic/kibana/assets/7416358/227fe648-d6b9-4788-8961-7369f6ed621a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yngrdyn yngrdyn yngrdyn approved these changes

Assignees

@achyutjhunjhunwala achyutjhunjhunwala

Labels
backport:skip This commit does not require backporting release_note:fix Team:APM All issues that need APM UI Team support v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[APM] Trace waterfall is not cleared when there are no results
6 participants
@achyutjhunjhunwala @apmmachine @elasticmachine @yngrdyn @kibana-ci @kibanamachine