[APM] Use transaction metrics for distribution charts #78484
Conversation
dgieselaar
added
Team:APM
|
Pinging @elastic/apm-ui (Team:apm) |
dgieselaar
force-pushed
the
distribution-transaction-metrics
branch
from
c3da520
to
e97ce9e
Compare
dgieselaar
force-pushed
the
distribution-transaction-metrics
branch
from
e97ce9e
to
0b15c03
Compare
|
@elasticmachine merge upstream |
1 similar comment
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
| @@ -69,10 +69,12 @@ export function useTransactionDistribution(urlParams: IUrlParams) { | |||
| // selected sample was not found. select a new one: | |||
| // sorted by total number of requests, but only pick | |||
| // from buckets that have samples | |||
| const bucketsSortedByPreference = response.buckets | |||
There was a problem hiding this comment.
From the name it sounds like the sorting depends on something. But they are always sorted by count, no?
| const bucketsSortedByPreference = response.buckets | |
| const bucketsSortedByCount = response.buckets |
or perhaps
| const bucketsSortedByPreference = response.buckets | |
| const bucketsSorted = response.buckets |
There was a problem hiding this comment.
Yes - will change it to bucketsSortedByCount.
| noHits: buckets.length === 0, | ||
| bucketSize, | ||
| buckets, | ||
| }; | ||
| } |
There was a problem hiding this comment.
I see the "fetcher" and "transform" have merged. I think the separation was nice in theory but perhaps it's not working out?
There was a problem hiding this comment.
It felt awkward moving back and forth between three files. As the fetch implementation was already significantly longer than the transform, I merged the three files into one.
| const url = `/api/apm/services/opbeans-java/transaction_groups/distribution?${qs.stringify({ | ||
| start, | ||
| end, | ||
| uiFilters, |
There was a problem hiding this comment.
Tiny nit: this might as well be inlined if we don't use uiFilters elsewhere (similar to transactionName and transactionType)
| uiFilters, | |
| uiFilters: {}, |
There was a problem hiding this comment.
cheers, nice little improvement. (will do it for start/end as well).
| }); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Thanks for adding an API test 👍
…-to-timeline * 'master' of github.com:elastic/kibana: (22 commits) update apm index pattern (elastic#78732) 78024: move transform out of dataset (elastic#78216) [QA][Code Coverage] Upload the coverage static site before ingestion (elastic#78695) [Discover] Make _source field not clickable (elastic#78698) [Fleet] Rename Ingest Manager => Fleet, Fleet => Agents in the UI (elastic#78685) [APM] Review feedback from distribution + transaction metrics (elastic#78752) [Ingest pipelines] Add ability to stop pipeline simulation (elastic#78183) [CSM] Fix core vital legend background (elastic#78273) [Usage Collection] [schema] Support spreads + `canvas` definition (elastic#78481) fix lodash imports (elastic#78456) [Maps] Add layer type preview icons (elastic#78650) [APM] Use transaction metrics for distribution charts (elastic#78484) [Uptime] Ml anomaly alert edit (elastic#76909) [ML] Limit exposing shared static code through ml/public/index.ts. (elastic#77745) making expression debug info serializable (elastic#78727) fix lodahs imports in app-arch code (elastic#78582) Make Field a React.lazy export (elastic#78483) [Security Solution] Improves detections tests (elastic#77295) [TSVB] Different field format on different series is ignored (elastic#78138) RFC: Improve saved object migrations (elastic#66056) ...
Closes #77717.