[APM] Alerting: Show preview as chart of threshold #84080

cauemarcondes · 2020-11-23T13:54:57Z

ogupte · 2020-12-09T21:42:29Z

x-pack/plugins/apm/public/components/alerting/chart_preview/index.tsx

+
+  // Make the maximum Y value either the actual max or 20% more than the threshold
+  const values = data.map((d) => d.y ?? 0);
+  const yMax = Math.max(Math.max(...values), threshold * 1.2);


could be simplified to Math.max(...values, threshold * 1.2);

elasticmachine · 2020-12-09T21:42:33Z

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

ogupte · 2020-12-09T22:00:57Z

x-pack/plugins/apm/public/components/alerting/chart_preview/index.tsx

+  const thresholdOpacity = 0.3;
+  const timestamps = data.map((d) => d.x);
+  const xMin = moment.utc(getMax(timestamps)).valueOf();
+  const xMax = moment.utc(getMin(timestamps)).valueOf();


Is moment needed? it seems like getMax(timestamps) & getMin(timestamps) would be sufficient.

sorenlouv · 2020-12-09T21:58:43Z

x-pack/plugins/apm/public/application/action_menu/alerting_popover_flyout.tsx

@@ -14,7 +14,7 @@ import { i18n } from '@kbn/i18n';
 import React, { useState } from 'react';
 import { IBasePath } from '../../../../../../src/core/public';
 import { AlertType } from '../../../common/alert_types';
-import { AlertingFlyout } from '../../components/alerting/AlertingFlyout';
+import { AlertingFlyout } from '../../components/alerting/alerting_flyout';


x-pack/plugins/apm/public/components/alerting/chart_preview/index.tsx

sorenlouv · 2020-12-09T22:04:42Z

x-pack/plugins/apm/public/components/alerting/helper.ts

+
+import datemath from '@elastic/datemath';
+
+export function windowToTimeRange(windowSize: number, windowUnit: string) {


windowToTimeRange seems a little vague. WDYT about getAbsoluteTimeRange?

Suggested change

export function windowToTimeRange(windowSize: number, windowUnit: string) {

export function getAbsoluteTimeRange(windowSize: number, windowUnit: string) {

x-pack/plugins/apm/public/components/alerting/error_count_alert_trigger/index.tsx

sorenlouv · 2020-12-09T22:17:28Z

x-pack/plugins/apm/public/components/alerting/transaction_error_rate_alert_trigger/index.tsx

+  const chartPreview = (
+    <ChartPreview
+      data={data}
+      yTickFormat={(d: any) => asPercent(d, 1)}


Can we narrow the type?

Suggested change

yTickFormat={(d: any) => asPercent(d, 1)}

yTickFormat={(d: number) => asPercent(d, 1)}

Agreed. It's taking a y value so probably number | null is better.

x-pack/plugins/apm/server/lib/alerts/chart_preview/get_transaction_duration.ts

sorenlouv · 2020-12-09T22:21:08Z

x-pack/plugins/apm/server/lib/alerts/chart_preview/get_transaction_error_count.ts

+    bool: {
+      filter: [
+        { range: rangeFilter(start, end) },
+        ,


extra comma?

This reverts commit 8d947d1.

smith · 2020-12-14T15:09:52Z

@elasticmachine merge upstream

kibanamachine · 2020-12-14T17:21:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 63604f2

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1726	1728	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	5.4MB	5.4MB	+57.8KB

Distributable file count

id	before	after	diff
`default`	47129	47893	+764

History

💚 Build #93618 succeeded edf8b6b
💚 Build #93518 succeeded ce0e3ae
💔 Build #93368 failed 8d947d1
💚 Build #93287 succeeded fb307e5
💔 Build #93176 failed 7656f15

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

tylersmalley · 2020-12-14T18:31:58Z

x-pack/plugins/apm/public/components/alerting/transaction_duration_alert_trigger/index.tsx

+import { useEnvironmentsFetcher } from '../../../hooks/use_environments_fetcher';
+import { callApmApi } from '../../../services/rest/createCallApmApi';
+import { getResponseTimeTickFormatter } from '../../shared/charts/transaction_charts/helper';
+import { useFormatter } from '../../shared/charts/transaction_charts/use_formatter';


This was removed in https://github.com/elastic/kibana/pull/84634/files#diff-6d6da4c7c8ceaea452f6660c70f214671b087f142f869f224514a3213b348427

tylersmalley · 2020-12-14T18:32:18Z

Reverting this PR, will post updates shortly.

This reverts commit 9986aff.

tylersmalley · 2020-12-14T18:35:25Z

Reverted due to failure in master.

master: d909a96
7.x Closed PR: #85819

There was a conflict with a line added to this PR that referenced a file removed in a PR merged an hour prior. #84080 (comment)

cauemarcondes added 5 commits November 24, 2020 09:31

adding chart preview for error rate

32931cd

adding chart preview

644fdf2

adding chart preview

a94872d

adding chart preview for error rate

f61f7c3

adding chart preview

fd50aa5

cauemarcondes force-pushed the apm-alerting-preview branch from b7a8c39 to fd50aa5 Compare November 24, 2020 08:34

cauemarcondes and others added 17 commits November 24, 2020 12:45

adding chart preview

b3f31c7

Merge remote-tracking branch 'upstream/master' into apm-alerting-preview

f88717e

preview and error rate changes

0c2c450

Error count alert trigger

78c05eb

Rename alerting flyout

8125874

More renames

4708452

some tests

69ff32b

Merge remote-tracking branch 'upstream/master' into apm-alerting-preview

d95c6d8

Merge remote-tracking branch 'upstream/master' into apm-alerting-preview

13bae9d

add empty api tests

6c01e05

transaction duration

e99622b

some stuff

0173d3b

fixes

ea5e925

Merge remote-tracking branch 'upstream/master' into apm-alerting-preview

7656f15

tests

092a0df

Merge remote-tracking branch 'upstream/master' into apm-alerting-preview

c090647

rm test

fb307e5

smith marked this pull request as ready for review December 9, 2020 21:37

smith requested a review from a team as a code owner December 9, 2020 21:37

ogupte reviewed Dec 9, 2020

View reviewed changes

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

ogupte reviewed Dec 9, 2020

View reviewed changes

sorenlouv approved these changes Dec 9, 2020

View reviewed changes

smith added release_note:enhancement v7.11.0 v8.0.0 labels Dec 10, 2020

smith added 4 commits December 10, 2020 00:18

Fixes based on feedback

a4064f0

Fix transaction route sort field

8d947d1

Merge remote-tracking branch 'upstream/master' into apm-alerting-preview

63fbfe5

Revert "Fix transaction route sort field"

ce0e3ae

This reverts commit 8d947d1.

smith self-assigned this Dec 10, 2020

Merge remote-tracking branch 'upstream/master' into apm-alerting-preview

edf8b6b

Merge branch 'master' into apm-alerting-preview

63604f2

smith merged commit 9986aff into elastic:master Dec 14, 2020

smith pushed a commit to smith/kibana that referenced this pull request Dec 14, 2020

[APM] Alerting: Show preview as chart of threshold (elastic#84080)

db64c28

smith mentioned this pull request Dec 14, 2020

[7.x] [APM] Alerting: Show preview as chart of threshold (#84080) #85819

Closed

tylersmalley reviewed Dec 14, 2020

View reviewed changes

tylersmalley pushed a commit that referenced this pull request Dec 14, 2020

Revert "[APM] Alerting: Show preview as chart of threshold (#84080)"

d909a96

This reverts commit 9986aff.

tylersmalley added the reverted label Dec 14, 2020

smith mentioned this pull request Dec 14, 2020

APM Alerts Preview charts #85868

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Alerting: Show preview as chart of threshold #84080

[APM] Alerting: Show preview as chart of threshold #84080

cauemarcondes commented Nov 23, 2020

ogupte Dec 9, 2020

elasticmachine commented Dec 9, 2020

ogupte Dec 9, 2020 •

edited

Loading

sorenlouv Dec 9, 2020

sorenlouv Dec 9, 2020

sorenlouv Dec 9, 2020

smith Dec 10, 2020

sorenlouv Dec 9, 2020

smith commented Dec 14, 2020

kibanamachine commented Dec 14, 2020

tylersmalley Dec 14, 2020

tylersmalley commented Dec 14, 2020

tylersmalley commented Dec 14, 2020


		import datemath from '@elastic/datemath';

		export function windowToTimeRange(windowSize: number, windowUnit: string) {

	export function windowToTimeRange(windowSize: number, windowUnit: string) {
	export function getAbsoluteTimeRange(windowSize: number, windowUnit: string) {

	yTickFormat={(d: any) => asPercent(d, 1)}
	yTickFormat={(d: number) => asPercent(d, 1)}

[APM] Alerting: Show preview as chart of threshold #84080

[APM] Alerting: Show preview as chart of threshold #84080

Conversation

cauemarcondes commented Nov 23, 2020

ogupte Dec 9, 2020

Choose a reason for hiding this comment

elasticmachine commented Dec 9, 2020

ogupte Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

sorenlouv Dec 9, 2020

Choose a reason for hiding this comment

sorenlouv Dec 9, 2020

Choose a reason for hiding this comment

sorenlouv Dec 9, 2020

Choose a reason for hiding this comment

smith Dec 10, 2020

Choose a reason for hiding this comment

sorenlouv Dec 9, 2020

Choose a reason for hiding this comment

smith commented Dec 14, 2020

kibanamachine commented Dec 14, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

History

tylersmalley Dec 14, 2020

Choose a reason for hiding this comment

tylersmalley commented Dec 14, 2020

tylersmalley commented Dec 14, 2020

ogupte Dec 9, 2020 •

edited

Loading