[AO] Compile the KQL expression on the server #162701

maryam-saeidi · 2023-07-28T11:20:56Z

Summary

This PR is related to this comment. Previously, we had both fitlerQueryText (unparsed KQL expression) and filterQuery (stringified JSON representation of the KQL expression). This makes the JSON body for creating a rule via the API confusing, as @simianhacker pointed out. In this PR, I've removed filterQuery and renamed fitlerQueryText to filterQuery to avoid confusion and parsed the filterQuery when needed.

After this change, you should only see filterQuery in the AAD and the rule definition with the user's value.

Rule definition	Alert document

🧪 How to test

Create a new threshold rule and add a filter for it
Make sure that all the rule functionalities that use filter work as expected
- Preview
- Rule execution
- Saved data in rule definition and alert document
Add an invalid filter query by editing the rule and make sure the validation works as expected (filter should be invalid state and save button should not work)

apmmachine · 2023-07-28T11:21:10Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

maryam-saeidi · 2023-07-31T07:47:57Z

...ty/public/components/threshold/components/custom_equation/custom_equation_editor.stories.tsx

@@ -22,7 +22,7 @@ import { MetricExpression } from '../../types';
 import { validateMetricThreshold } from '../validation';

 export default {
-  title: 'infra/alerting/CustomEquationEditor',
+  title: 'app/Alerts/CustomEquationEditor',


Storybook is not working properly, created a separate ticket for fixing it (#162720)

elasticmachine · 2023-07-31T07:56:24Z

Pinging @elastic/actionable-observability (Team: Actionable Observability)

fkanout

Tested locally, and the rule has been evaluated correctly, fires alerts, and the filterQuery data are available in the alert and the rule doc thanks @maryam-saeidi !

fkanout · 2023-07-31T13:07:43Z

x-pack/plugins/observability/server/lib/rules/threshold/lib/metric_query.ts

@@ -25,7 +26,13 @@ const getParsedFilterQuery: (filterQuery: string | undefined) => Array<Record<st
  filterQuery
 ) => {
  if (!filterQuery) return [];
-  return [JSON.parse(filterQuery)];
+
+  try {


@maryam-saeidi, this is where compile the filterQuery on the server side instead of convertKueryToElasticSearchQuery in the frontend, right?

simianhacker · 2023-08-01T13:43:27Z

x-pack/plugins/observability/server/lib/rules/threshold/register_threshold_rule_type.ts

@@ -132,7 +132,7 @@ export function thresholdRuleType(
          groupBy: schema.maybe(schema.oneOf([schema.string(), schema.arrayOf(schema.string())])),
          filterQuery: schema.maybe(
            schema.string({
-              validate: validateIsStringElasticsearchJSONFilter,
+              validate: validateKQLStringFilter,


@kdelemme We should add this to our schemas for anywhere we accept KQL.

@maryam-saeidi Can we add this to the filter attribute of the customCriterion.customMetrics object as well?

simianhacker

Everything looks good code-wise and functionality. Can we add the validateKQLStringFilter to the schema for the customCriterion.customMetrics for the filter attributes?

simianhacker

LGTM

kibana-ci · 2023-08-01T16:35:48Z

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id	before	after	diff
`observability`	1.0MB	1.0MB	-264.0B

Page load bundle

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

id	before	after	diff
`observability`	58.5KB	58.7KB	+201.0B

History

💚 Build #145833 succeeded 2dfd27d
💚 Build #145580 succeeded 3303086
💔 Build #145521 failed e391739
💔 Build #145492 failed ae14855

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

cc @maryam-saeidi

maryam-saeidi · 2023-08-02T06:37:55Z

x-pack/plugins/observability/server/lib/rules/threshold/utils.ts

-      return undefined;
-    }
-    return errorMessage;
+    kbnBuildEsQuery(undefined, [{ query: value, language: 'kuery' }], []);


I asked about the differences between fromKueryExpression and kbnBuildEsQuery, here is the answer: (link)

I don’t think there is a difference. buildEsQuery calls those methods under the hood
It’s what you get when you call fromKueryExpression, and what you pass to toElasticsearchQuery

Remove filterQuery in favor of filterQueryText

ae14855

maryam-saeidi self-assigned this Jul 28, 2023

maryam-saeidi added the release_note:skip Skip the PR/issue when compiling release notes label Jul 28, 2023

maryam-saeidi added 4 commits July 28, 2023 14:39

Rename fitlerQueryText to fitlerQuery

0c4149b

Merge branch 'main' into 159017-compile-kql-on-server

e391739

Fix test and type

714e679

Merge branch 'main' into 159017-compile-kql-on-server

3303086

maryam-saeidi marked this pull request as ready for review July 29, 2023 05:10

maryam-saeidi requested a review from a team as a code owner July 29, 2023 05:10

maryam-saeidi requested a review from simianhacker July 29, 2023 16:48

maryam-saeidi commented Jul 31, 2023

View reviewed changes

maryam-saeidi added 2 commits July 31, 2023 09:55

Remove extra comment

43b1a75

Merge branch 'main' into 159017-compile-kql-on-server

2dfd27d

maryam-saeidi added the Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" label Jul 31, 2023

fkanout self-requested a review July 31, 2023 13:12

fkanout approved these changes Jul 31, 2023

View reviewed changes

simianhacker reviewed Aug 1, 2023

View reviewed changes

simianhacker requested changes Aug 1, 2023

View reviewed changes

maryam-saeidi added 2 commits August 1, 2023 17:08

Use validateKQLStringFilter for customMetrics count filter

2fd902d

Merge branch 'main' into 159017-compile-kql-on-server

6fe9f26

simianhacker self-requested a review August 1, 2023 16:13

simianhacker approved these changes Aug 1, 2023

View reviewed changes

maryam-saeidi merged commit d0d3127 into elastic:main Aug 2, 2023

kibanamachine added v8.10.0 backport:skip This commit does not require backporting labels Aug 2, 2023

maryam-saeidi commented Aug 2, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AO] Compile the KQL expression on the server #162701

[AO] Compile the KQL expression on the server #162701

maryam-saeidi commented Jul 28, 2023 •

edited

Loading

apmmachine commented Jul 28, 2023

maryam-saeidi Jul 31, 2023

elasticmachine commented Jul 31, 2023

fkanout left a comment •

edited

Loading

fkanout Jul 31, 2023

maryam-saeidi Jul 31, 2023

simianhacker Aug 1, 2023

simianhacker Aug 1, 2023 •

edited

Loading

maryam-saeidi Aug 1, 2023

maryam-saeidi Aug 1, 2023

simianhacker left a comment

simianhacker left a comment

kibana-ci commented Aug 1, 2023

maryam-saeidi Aug 2, 2023

[AO] Compile the KQL expression on the server #162701

[AO] Compile the KQL expression on the server #162701

Conversation

maryam-saeidi commented Jul 28, 2023 • edited Loading

Summary

🧪 How to test

apmmachine commented Jul 28, 2023

🤖 GitHub comments

maryam-saeidi Jul 31, 2023

Choose a reason for hiding this comment

elasticmachine commented Jul 31, 2023

fkanout left a comment • edited Loading

Choose a reason for hiding this comment

fkanout Jul 31, 2023

Choose a reason for hiding this comment

maryam-saeidi Jul 31, 2023

Choose a reason for hiding this comment

simianhacker Aug 1, 2023

Choose a reason for hiding this comment

simianhacker Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

maryam-saeidi Aug 1, 2023

Choose a reason for hiding this comment

maryam-saeidi Aug 1, 2023

Choose a reason for hiding this comment

simianhacker left a comment

Choose a reason for hiding this comment

simianhacker left a comment

Choose a reason for hiding this comment

kibana-ci commented Aug 1, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

maryam-saeidi Aug 2, 2023

Choose a reason for hiding this comment

maryam-saeidi commented Jul 28, 2023 •

edited

Loading

fkanout left a comment •

edited

Loading

simianhacker Aug 1, 2023 •

edited

Loading