[APM] Add error rates to Service Map popovers #69520

smith · 2020-06-18T15:07:44Z

Change the average errors per minute to error rate on the service map popover.

Make the getErrorRate function used in the error rate charts return an average property that can be used in the error rate chart and on the popovers.

Call that function in the API for the service map stats.

Rename all of the "metric" wording of the methods and classes to use "stats" instead, since "metric" is a very overloaded term.

Update the popovers to use the environment of the node being selected rather than the environment of the environment switcher.

Add the popover to Storybook.

Fixes #68160.

Make the `getErrorRate` function used in the error rate charts additionally take `service.environment` as a filter and have it return the `average` of the values. Call that function in the API for the service map metrics. Fixes elastic#68160.

elasticmachine · 2020-06-18T15:07:50Z

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

x-pack/plugins/apm/server/lib/service_map/get_service_map_service_node_info.ts

x-pack/plugins/apm/server/lib/errors/get_error_rate.ts

…lter out NaN from average.

…-rate

smith

There's an unused prop that can be removed, but other than that looks good. Thanks so much for picking this up. I can't approve since I'm the original author.

x-pack/plugins/apm/public/components/app/ServiceMap/Popover/Contents.tsx

x-pack/plugins/apm/public/components/app/ServiceMap/Popover/ServiceStatsFetcher.tsx

ogupte · 2020-07-14T18:00:28Z

x-pack/plugins/apm/server/lib/helpers/convert_ui_filters/get_parsed_ui_filters.ts

+import { Logger } from 'src/core/server';
+import { UIFilters } from '../../../../typings/ui_filters';
+
+export function getParsedUiFilters({


perhaps this should be used here as well:

kibana/x-pack/plugins/apm/server/lib/helpers/setup_request.ts

Line 26 in ebc0b86

const uiFilters = JSON.parse(uiFiltersEncoded);

ogupte · 2020-07-14T18:06:42Z

x-pack/plugins/apm/server/lib/service_map/get_service_map_service_node_info.ts

+  const setupWithBlankUiFilters = {
+    ...setup,
+    uiFiltersES: getEnvironmentUiFilterES(environment),
  };


i'm not sure setupWithBlankUiFilters is required: it could be redundant with setup.uiFiltersES. which should be part of the setup object (typed with SetupUIFilters).

I think it is @ogupte , since setup.uiFiltersES has the service.name of the route. And it's not what you want on Service Maps, you want the service.name from the node selected, right? So If we don't remove it, we'll have two service.name filters one from the route and another from the node.

x-pack/plugins/translations/translations/zh-CN.json

ogupte · 2020-07-14T18:09:49Z

x-pack/plugins/translations/translations/zh-CN.json

-    "xpack.apm.serviceMap.avgCpuUsagePopoverMetric": "CPU 使用（平均）",
-    "xpack.apm.serviceMap.avgErrorsPerMinutePopoverMetric": "每分钟错误数（平均）",
-    "xpack.apm.serviceMap.avgMemoryUsagePopoverMetric": "内存使用（平均）",
-    "xpack.apm.serviceMap.avgReqPerMinutePopoverMetric": "每分钟请求数（平均）",
-    "xpack.apm.serviceMap.avgTransDurationPopoverMetric": "事务持续时间（平均）",


I think only the translation for errors would have changed, so the others should only update their ids

ogupte

few comments, but looks good, and verified with local testing

smith · 2020-07-14T23:07:29Z

retest

sorenlouv · 2020-07-15T08:28:44Z

@elasticmachine merge upstream

sorenlouv · 2020-07-15T08:33:24Z

x-pack/test/apm_api_integration/trial/tests/service_maps.ts

 import expect from '@kbn/expect';
 import { FtrProviderContext } from '../../common/ftr_provider_context';

 export default function serviceMapsApiTests({ getService }: FtrProviderContext) {
  const supertest = getService('supertest');
  const esArchiver = getService('esArchiver');

-  const start = encodeURIComponent('2020-06-29T06:45:00.000Z');
-  const end = encodeURIComponent('2020-06-29T06:49:00.000Z');


Why not keep this?

x-pack/test/apm_api_integration/trial/tests/service_maps.ts

sorenlouv · 2020-07-15T08:34:48Z

x-pack/plugins/apm/server/routes/service_map.ts

    return getServiceMapServiceNodeInfo({
      setup,
      serviceName,
-      environment,
+      uiFilters,


Thanks for fixing this. Glad that we are back to using the uiFilters

kibanamachine · 2020-07-15T10:06:45Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4a0a421

Build metrics

async chunks size

id	value	diff	baseline
apm	5.2MB	-155.0B	5.2MB

miscellaneous assets size

id	value	diff	baseline
upgradeAssistant	22.5KB	-15.0B	22.6KB

History

💚 Build #61817 succeeded c623b90
💔 Build #61724 failed c78892b
💔 Build #61620 failed c78892b
💔 Build #61530 failed ebc0b86
💔 Build #61503 failed f7965a1

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

* master: [APM] Add error rates to Service Map popovers (elastic#69520) [Security Solution][Detection Engine] - Update exceptions logic (elastic#71512) [Security Solution] Full screen timeline, Collapse event (elastic#71786) [Security Solution][Exception Modal] Create endpoint exception list if it doesn't already exist (elastic#71807) [Detection Rules] Add 7.9 rules (elastic#71808) [Search] Add telemetry for data plugin search service (elastic#70677) Add @elastic/safer-lodash-set as an alternative to lodash.set (elastic#67452) [tests] Temporarily skipped to promote snapshot

Make the `getErrorRate` function used in the error rate charts additionally take `service.environment` as a filter and have it return the `average` of the values. Call that function in the API for the service map metrics. Fixes elastic#68160. Co-authored-by: cauemarcondes <[email protected]>

* master: [APM] Add error rates to Service Map popovers (elastic#69520) [Security Solution][Detection Engine] - Update exceptions logic (elastic#71512) [Security Solution] Full screen timeline, Collapse event (elastic#71786) [Security Solution][Exception Modal] Create endpoint exception list if it doesn't already exist (elastic#71807) [Detection Rules] Add 7.9 rules (elastic#71808) [Search] Add telemetry for data plugin search service (elastic#70677) Add @elastic/safer-lodash-set as an alternative to lodash.set (elastic#67452) [tests] Temporarily skipped to promote snapshot

Make the `getErrorRate` function used in the error rate charts additionally take `service.environment` as a filter and have it return the `average` of the values. Call that function in the API for the service map metrics. Fixes #68160. Co-authored-by: cauemarcondes <[email protected]> Co-authored-by: cauemarcondes <[email protected]>

Make the `getErrorRate` function used in the error rate charts additionally take `service.environment` as a filter and have it return the `average` of the values. Call that function in the API for the service map metrics. Fixes elastic#68160. Co-authored-by: cauemarcondes <[email protected]>

Make the `getErrorRate` function used in the error rate charts additionally take `service.environment` as a filter and have it return the `average` of the values. Call that function in the API for the service map metrics. Fixes #68160. Co-authored-by: cauemarcondes <[email protected]> Co-authored-by: cauemarcondes <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

smith requested a review from a team as a code owner June 18, 2020 15:07

botelastic bot added the Team:APM All issues that need APM UI Team support label Jun 18, 2020

smith added v7.9.0 release_note:enhancement labels Jun 18, 2020

smith added 3 commits June 18, 2020 12:32

Remove unused import from chart

8fa2b55

Merge remote-tracking branch 'upstream/master' into nls/pop-error-rate

4ebfd44

Merge remote-tracking branch 'upstream/master' into nls/pop-error-rate

ec1d918