-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Share components across Service overview and transactions. Add time comparison to the Transactions page #107299
[APM] Share components across Service overview and transactions. Add time comparison to the Transactions page #107299
Conversation
@elasticmachine merge upstream |
Pinging @elastic/apm-ui (Team:apm) |
…marcondes/kibana into apm-transaction-overview-comparison
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Amazing how fast it seems now on the Transactions list page because they're shared?
It'll definitely be faster now, since it uses the same API and the result is cached. |
...ins/apm/public/components/app/service_overview/service_overview_transactions_table/index.tsx
Outdated
Show resolved
Hide resolved
@@ -94,137 +169,47 @@ function getItemsWithRelativeImpact( | |||
item.sum !== null && item.sum !== undefined | |||
? ((item.sum - min) / (max - min)) * 100 || 0 | |||
: 0, | |||
p95: item.p95, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we use p95 anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, the current table doesn't have it. #91848 (comment)
<TransactionList | ||
isLoading={transactionListStatus === 'loading'} | ||
items={transactionListData.items || []} | ||
<ServiceOverviewTransactionsTable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it still called ServiceOverviewTransactionsTable
if it's not on the service overview?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I actually forgot to move it to the shared
folder and rename it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it here: b8209e6
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
…time comparison to the Transactions page (elastic#107299) * adding comparison to transactions pages * adding new transactions table * adding throughput * refactoring transacon group api * adding missing filter * fixing i18n * fixing tests * addressing PR comments * moving table to shared folder Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…time comparison to the Transactions page (#107299) (#107576) * adding comparison to transactions pages * adding new transactions table * adding throughput * refactoring transacon group api * adding missing filter * fixing i18n * fixing tests * addressing PR comments * moving table to shared folder Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>
…time comparison to the Transactions page (elastic#107299) * adding comparison to transactions pages * adding new transactions table * adding throughput * refactoring transacon group api * adding missing filter * fixing i18n * fixing tests * addressing PR comments * moving table to shared folder Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
closes #91848
closes #106319
I'm closing both issues in one PR. Not ideal, but to be able to add comparison to the transactions pages I'll need to use the same components as the service overview.