Add group-by feature in APM rules #155001

benakansara · 2023-04-16T17:28:59Z

Summary

Adds group-by dropdown in the following APM rules.

APM Latency threshold (Preselected fields: service.name, service.environment, transaction.type)
APM Failed transaction rate (Preselected fields: service.name, service.environment, transaction.type)
APM Error count threshold (Preselected fields: service.name, service.environment)

The preselected fields cannot be removed by user. The transaction.name field is selectable by user from the group-by dropdown.

Reason message is updated to include group key instead of only service name:

Update reason message of the APM rules to include group key instead of only service name #155011

The transaction.name is added to the alert document:

Add transaction.name in alert document for APM rules #154543

The transaction.name action variable is added in UI:

Add action variable transactionName in APM rules #154545

The transaction.name is added to the context of active alert notifications:

Add transactionName in the context of active alerts of APM rules #154547

There is an additional field in group-by dropdown for Error count threshold rule: #155633

error.grouping_key

Fixes

Latency threshold rule schedules alerts with same ID #154818

Update on Alert Id

The alert Id is updated for all 3 rules. The new Id is generated from the group key. This is to avoid issues similar to #154818 where alerts are scheduled with same ID. Example of the new alert Ids - opbeans-java_development_request_GET /flaky, opbeans-java_development_GET /fail

Out of scope of this PR

Updating the preview chart based on selected group by fields

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

Release note

As the alert Id is updated for the APM Latency threshold rule, APM Failed transaction rate rule and APM Error count rule, the existing alerts, if any, will be recovered, and new alerts will be fired in place of them.

apmmachine · 2023-04-16T17:29:15Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2023-04-17T11:56:30Z

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

benakansara · 2023-04-17T13:46:34Z

/oblt-deploy

kdelemme

I did a first pass, small neats. Going to tests further

x-pack/plugins/apm/common/rules/schema.ts

x-pack/plugins/apm/public/components/alerting/ui_components/apm_rule_group_by.tsx

...ck/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts

benakansara · 2023-04-19T16:18:21Z

/oblt-deploy

benakansara · 2023-04-19T22:18:15Z

/oblt-deploy

…/register_error_count_rule_type.ts Co-authored-by: Søren Louv-Jansen <[email protected]>

benakansara · 2023-04-25T08:47:59Z

Hmm.. Yeah, you could do that. But alternatively you could hash the message like:

I am fine with either way (removing for now or using hash). My only concern is that if we need more discussion around alert ID, we should wait till it is clear.

dgieselaar · 2023-04-25T09:47:11Z

...ck/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts

@@ -88,6 +88,9 @@ export function registerErrorCountRuleType({
      minimumLicenseRequired: 'basic',
      isExportable: true,
      executor: async ({ params: ruleParams, services, spaceId }) => {
+        const predefinedGroupby = [SERVICE_NAME, SERVICE_ENVIRONMENT];
+        const allGroupbyFields = ruleParams.groupBy ?? predefinedGroupby;


Does this mean that users can (maybe theoretically?) force the rule to not use service.name and service.environment as grouping keys?

No. This check is to support existing rules when upgrading Kibana since groupBy was not present before so it's value will be undefined. If user opens the rule in UI and saves it, this will be updated.

Users can (and do) use the API directly though. I think these kinds of things should ideally be in the server. Part of it in the browser and part of it in the server is a bit risky IMHO. But not a blocker.

I see, I have updated the code to always include predefined group-by fields.

...ck/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts

benakansara · 2023-04-25T10:35:40Z

@dgieselaar @sqren
I am going to remove error.grouping_name entirely (UI, logic) for now. I will add it back when alert ID part is clear.

update: removed error.grouping_name related functionality

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts

x-pack/test/apm_api_integration/tests/alerts/transaction_duration.spec.ts

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts

sorenlouv

Just a few nits left. LGTM and thanks for all the help!
APM alerting is improving rapidly these days!

x-pack/test/apm_api_integration/tests/alerts/error_count_threshold.spec.ts

kibana-ci · 2023-04-25T13:37:04Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 1b1cbe6

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1322	1323	+1

Async chunks

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

id	before	after	diff
`apm`	3.5MB	3.5MB	+4.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`apm`	32.3KB	32.3KB	+10.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	17	19	+2
`securitySolution`	395	398	+3
total			+5

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	18	20	+2
`securitySolution`	475	478	+3
total			+5

History

💔 Build #122830 failed 48a22c5
💚 Build #122703 succeeded b7badaa
💚 Build #122604 succeeded c126029
💔 Build #122532 failed 5fd7ae4
💔 Build #122485 failed 6c553d4

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

cc @benakansara

sorenlouv · 2023-04-26T07:37:54Z

...ver/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts

+      const allGroupbyFields = Array.from(
+        new Set([...predefinedGroupby, ...(ruleParams.groupBy ?? [])])
+      );


nit: with lodash this can be simplified like:

const allGroupbyFields = union(predefinedGroupBy, ruleParams.groupBy)

(also: there is a consistency with "group by". Sometimes it's written as "groupby" other times as "groupBy". I suggest we consistently use the latter.

I will address this in a new PR.

add group alerts by feature in apm rules

bfaf0cb

benakansara added backport:skip This commit does not require backporting release_note:feature Makes this part of the condensed release notes v8.8.0 labels Apr 16, 2023

benakansara self-assigned this Apr 16, 2023

benakansara added 7 commits April 16, 2023 20:00

fix ci errors

d790929

fix translations errors

e351476

fix tests

e76bc4f

fix tests

0ca0259

fix tests

02053b7

fix integration tests

66be550

update snapshot

aebab24

benakansara changed the title ~~Add group by feature in APM rules~~ Add group by options in APM rules Apr 17, 2023

benakansara requested a review from a team April 17, 2023 09:01

benakansara changed the title ~~Add group by options in APM rules~~ Add group-by feature in APM rules Apr 17, 2023

benakansara marked this pull request as ready for review April 17, 2023 11:55

benakansara requested a review from a team as a code owner April 17, 2023 11:55

Merge branch 'main' into feat/add-groupby-in-apm-rules

f295ce7

botelastic bot added the Team:APM All issues that need APM UI Team support label Apr 17, 2023

Merge branch 'main' into feat/add-groupby-in-apm-rules

4419fe8

sorenlouv requested a review from kpatticha April 17, 2023 22:57

kdelemme reviewed Apr 18, 2023

View reviewed changes

add transaction.name in AAD mapping, add transactionName in UI

a6cf45d

benakansara mentioned this pull request Apr 19, 2023

[APM] Add transaction name filter in latency threshold rule #154241

Merged

1 task

update type

bc90ecf

benakansara and others added 3 commits April 25, 2023 10:27

Update x-pack/plugins/apm/server/routes/alerts/rule_types/error_count…

42127e0

…/register_error_count_rule_type.ts Co-authored-by: Søren Louv-Jansen <[email protected]>

update tests

6f8e6cc

index environment in alert doc also when not defined

48a22c5

dgieselaar reviewed Apr 25, 2023

View reviewed changes

...ck/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts Show resolved Hide resolved

benakansara added 2 commits April 25, 2023 11:54

update snapshot

050dbf5

always adding predefined group bys

546f0cb

remove error.grouping_name groupby in error count rule

009ead9

sorenlouv reviewed Apr 25, 2023

View reviewed changes

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts Outdated Show resolved Hide resolved

sorenlouv reviewed Apr 25, 2023

View reviewed changes

x-pack/test/apm_api_integration/tests/alerts/transaction_duration.spec.ts Outdated Show resolved Hide resolved

sorenlouv reviewed Apr 25, 2023

View reviewed changes

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts Show resolved Hide resolved

rename back action variables

1bde480

sorenlouv approved these changes Apr 25, 2023

View reviewed changes

updated tests

69af32b

benakansara enabled auto-merge (squash) April 25, 2023 12:17

updated tests

b662a15

benakansara disabled auto-merge April 25, 2023 12:26

sorenlouv reviewed Apr 25, 2023

View reviewed changes

x-pack/test/apm_api_integration/tests/alerts/error_count_threshold.spec.ts Outdated Show resolved Hide resolved

remove hardcoded error grouping key

1b1cbe6

benakansara enabled auto-merge (squash) April 25, 2023 12:53

benakansara merged commit ddd09ac into elastic:main Apr 25, 2023

benakansara mentioned this pull request Apr 25, 2023

Latency threshold rule schedules alerts with same ID #154818

Closed

sorenlouv reviewed Apr 26, 2023

View reviewed changes

kpatticha mentioned this pull request Apr 26, 2023

[REQUEST]: Document new functionality for APM alerts & rules elastic/observability-docs#2878

Closed

benakansara mentioned this pull request Apr 27, 2023

UX to notify user about possibly high number of alert notifications #156070

Open

sorenlouv mentioned this pull request Jul 11, 2023

[APM] Ability to use filtering and breakdowns in APM rules #142302

Closed

benakansara deleted the feat/add-groupby-in-apm-rules branch October 23, 2023 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add group-by feature in APM rules #155001

Add group-by feature in APM rules #155001

benakansara commented Apr 16, 2023 •

edited

Loading

apmmachine commented Apr 16, 2023

elasticmachine commented Apr 17, 2023

benakansara commented Apr 17, 2023

kdelemme left a comment

benakansara commented Apr 19, 2023

benakansara commented Apr 19, 2023

benakansara commented Apr 25, 2023

dgieselaar Apr 25, 2023

benakansara Apr 25, 2023

dgieselaar Apr 25, 2023

benakansara Apr 25, 2023

benakansara commented Apr 25, 2023 •

edited

Loading

sorenlouv left a comment

kibana-ci commented Apr 25, 2023

ESLint disabled line counts

Total ESLint disabled count

sorenlouv Apr 26, 2023 •

edited

Loading

benakansara Apr 26, 2023

Add group-by feature in APM rules #155001

Add group-by feature in APM rules #155001

Conversation

benakansara commented Apr 16, 2023 • edited Loading

Summary

Fixes

Update on Alert Id

Out of scope of this PR

Checklist

Release note

apmmachine commented Apr 16, 2023

🤖 GitHub comments

elasticmachine commented Apr 17, 2023

benakansara commented Apr 17, 2023

kdelemme left a comment

Choose a reason for hiding this comment

benakansara commented Apr 19, 2023

benakansara commented Apr 19, 2023

benakansara commented Apr 25, 2023

dgieselaar Apr 25, 2023

Choose a reason for hiding this comment

benakansara Apr 25, 2023

Choose a reason for hiding this comment

dgieselaar Apr 25, 2023

Choose a reason for hiding this comment

benakansara Apr 25, 2023

Choose a reason for hiding this comment

benakansara commented Apr 25, 2023 • edited Loading

sorenlouv left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 25, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

sorenlouv Apr 26, 2023 • edited Loading

Choose a reason for hiding this comment

benakansara Apr 26, 2023

Choose a reason for hiding this comment

benakansara commented Apr 16, 2023 •

edited

Loading

benakansara commented Apr 25, 2023 •

edited

Loading

sorenlouv Apr 26, 2023 •

edited

Loading