Repository navigation
[APM] Service overview: Instances table - #85770
Conversation
…iew-instances-table
…iew-instances-table
|
Pinging @elastic/apm-ui (Team:apm) |
…iew-instances-table
smith
left a comment
There was a problem hiding this comment.
Looks great.
It would be very cool if you also removed the box for the latency distribution chart and made this full-width.
| : serviceNodeName; | ||
|
|
||
| const link = | ||
| agentName === 'java' ? ( |
There was a problem hiding this comment.
We do have an isJavaAgentName function (it does the same thing but returns agentName is 'java' in common/agent_name.ts.
| <EuiTitle size="xs"> | ||
| <h2> | ||
| {i18n.translate('xpack.apm.serviceOverview.instancesTableTitle', { | ||
| defaultMessage: 'All instances', |
There was a problem hiding this comment.
I know this matches the mockup, but why is it not just "Instances"? @formgeist
| ), | ||
| render: (_, item) => { | ||
| const { serviceNodeName } = item; | ||
| const isMissing = serviceNodeName === SERVICE_NODE_NAME_MISSING; |
There was a problem hiding this comment.
nit
| const isMissing = serviceNodeName === SERVICE_NODE_NAME_MISSING; | |
| const isMissingNodeName = serviceNodeName === SERVICE_NODE_NAME_MISSING; |
| width: px(unit * 10), | ||
| render: (_, { throughput }) => { | ||
| return ( | ||
| <SparkPlotWithValueLabel |
There was a problem hiding this comment.
tangential: SparkPlotWithValueLabel seems like it should simply be:
<SparkPlot />Then, if we want to hide the label value:
<SparkPlot showValueLabel={false} />or perhaps:
<SparkPlot hideValueLabel={true} />|
|
||
| interface Response { | ||
| status: number; | ||
| body: APIReturnType<'GET /api/apm/services/{serviceName}/service_overview_instances'>; |
There was a problem hiding this comment.
Were you not the slightest tempted to write a client when you did this? :p
There was a problem hiding this comment.
I am 😬 but I would like someone else to pick that up.
| ) : ( | ||
| <MetricOverviewLink | ||
| serviceName={serviceName} | ||
| mergeQuery={(query) => ({ |
There was a problem hiding this comment.
Instead of mergeQuery can we just update MetricOverviewLink to handle the merge:
<APMLink
path={`/services/${serviceName}/metrics`}
query={{
...query,
...pickKeys(urlParams as APMQueryParams, ...persistedFilters)
}}
{...rest}
/>There was a problem hiding this comment.
I've added mergeQuery to APMLink so it's reusable. I've also made it a function to leave ordering etc up to the consumer. One might want to set a default value, or override a parameter, or simply remove one.
* master: (66 commits) [Alerting] fixes broken Alerting Example plugin (elastic#85774) [APM] Service overview instances table (elastic#85770) [Security Solution] Unskip timeline creation Cypress test (elastic#85871) properly recognize enterprise licenses (elastic#85849) [SecuritySolution][Detections] Adds SavedObject persistence to Signals Migrations (elastic#85690) [TSVB] Fix functional tests flakiness and unskip them (elastic#85388) [Fleet] Change permissions for Fleet enroll role (elastic#85802) Gauge visualization can no longer be clicked to filter on values since Kibana 7.10.0 (elastic#84768) [Security Solution][Detections] Add alert source to detection rule action context (elastic#85488) [Discover] Don't display hide/show button for histogram when there's no time filter (elastic#85424) skip flaky suite (elastic#78553) License checks for alerts plugin (elastic#85649) skip flaky suite (elastic#84992) skip 'query return results valid for scripted field' elastic#78553 Allow action types to perform their own mustache variable escaping in parameter templates (elastic#83919) [ML] More machine learning links in doc_links_service.ts (elastic#85365) Removed Alerting & Event Log deprecated fields that should not be using (elastic#85652) Closes elastic#79995 by adding new tab in transaction details to show related trace logs. (elastic#85859) Fix outdated jest snapshot [Maps] Surface on prem EMS (elastic#85729) ...
💚 Build SucceededMetrics [docs]Async chunks
Distributable file count
History
To update your PR or re-run it, just comment with: |
Closes #81721.