Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replaces kqlFilter with searchConfiguration and uses Unified Search bar in APM rules #164540

Merged
merged 10 commits into from
Aug 26, 2023
Merged

Replaces kqlFilter with searchConfiguration and uses Unified Search bar in APM rules #164540

merged 10 commits into from
Aug 26, 2023

Conversation

benakansara
Copy link
Contributor

@benakansara benakansara commented Aug 23, 2023 •
edited by kibanamachine
Loading

Fixes #163781

  • kqlFilter which was string is replaced with searchConfiguration
  • Replaces KQL filter component with Unified Search bar
  • Removed "Technical Preview" from the "Filter" label

@apmmachine
Copy link
Contributor

🤖 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!)

@benakansara benakansara self-assigned this Aug 23, 2023
@benakansara benakansara added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.10.0 Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" Team:APM - DEPRECATED Use Team:obs-ux-infra_services. labels Aug 23, 2023
@benakansara benakansara marked this pull request as ready for review August 23, 2023 09:20
@benakansara benakansara requested a review from a team as a code owner August 23, 2023 09:20
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

sorenlouv
sorenlouv reviewed Aug 23, 2023
View reviewed changes
x-pack/plugins/apm/public/components/alerting/rule_types/error_count_rule_type/index.tsx
@@ -119,7 +123,7 @@ export function ErrorCountRuleType(props: Props) {
params.serviceName,
params.errorGroupingKey,
params.groupBy,
params.kqlFilter,
params.searchConfiguration,
Copy link
Member

@sorenlouv sorenlouv Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be problematic. Before kqlFilter was a string, so the identity only changes if the string changes - and thus the preview request will only be made when the search query changes. Now, the preview request will be made every time params.searchConfiguration is defined (happens on every render afaict).

This will fix the problem:

Suggested change
params.searchConfiguration,
params.searchConfiguration.query,
params.searchConfiguration.kuery,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just seen that query can be an object, in which case the above won't work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I follow. I checked the preview request, it is made only when search query changes. Could you give example of other instances when preview request is made?

Copy link
Member

@sorenlouv sorenlouv Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a general problem when using non-primitives as dependencies. There are various blog posts on the topic:

It's not necessarily a problem but it's a foot gun and important to be aware of.

benakansara added a commit to benakansara/kibana that referenced this pull request Aug 26, 2023
@benakansara
Replaces kqlFilter with searchConfiguration and uses Unified Search b…
e0ac9f7 
…ar in APM rules (elastic#164540)

Fixes elastic#163781

- `kqlFilter` which was string is replaced with `searchConfiguration`
- Replaces KQL filter component with Unified Search bar
- Removed "Technical Preview" from the "Filter" label

(cherry picked from commit b88235a)

# Conflicts:
#	x-pack/test/apm_api_integration/tests/alerts/error_count_threshold.spec.ts
#	x-pack/test/apm_api_integration/tests/alerts/transaction_duration.spec.ts
@benakansara
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@benakansara benakansara mentioned this pull request Aug 27, 2023
Closed
@benakansara
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@benakansara benakansara added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 27, 2023
benakansara added a commit to benakansara/kibana that referenced this pull request Aug 27, 2023
@benakansara
Replaces kqlFilter with searchConfiguration and uses Unified Search b…
8afd505 
…ar in APM rules (elastic#164540)

Fixes elastic#163781

- `kqlFilter` which was string is replaced with `searchConfiguration`
- Replaces KQL filter component with Unified Search bar
- Removed "Technical Preview" from the "Filter" label

(cherry picked from commit b88235a)
@benakansara
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@benakansara benakansara removed auto-backport Deprecated - use backport:version if exact versions are needed backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.10.0 labels Aug 27, 2023
@kibanamachine kibanamachine added backport:skip This commit does not require backporting labels Aug 27, 2023
@benakansara benakansara added auto-backport Deprecated - use backport:version if exact versions are needed v8.10.0 and removed backport:skip This commit does not require backporting auto-backport Deprecated - use backport:version if exact versions are needed v8.10.0 labels Aug 27, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 27, 2023
@benakansara benakansara added auto-backport Deprecated - use backport:version if exact versions are needed v8.10.0 labels Aug 27, 2023
@kibanamachine kibanamachine mentioned this pull request Aug 27, 2023
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine pushed a commit that referenced this pull request Aug 27, 2023
@benakansara
Replaces kqlFilter with searchConfiguration and uses Unified Search b…
3945e0b 
…ar in APM rules (#164540)

Fixes #163781

- `kqlFilter` which was string is replaced with `searchConfiguration`
- Replaces KQL filter component with Unified Search bar
- Removed "Technical Preview" from the "Filter" label

(cherry picked from commit b88235a)
kibanamachine added a commit that referenced this pull request Aug 27, 2023
@kibanamachine @benakansara
[8.10] Replaces kqlFilter with searchConfiguration and uses Unified S…
bc1e560 
…earch bar in APM rules (#164540) (#164938)

# Backport

This will backport the following commits from `main` to `8.10`:
- [Replaces kqlFilter with searchConfiguration and uses Unified Search
bar in APM rules
(#164540)](#164540)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Bena
Kansara","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-26T13:55:05Z","message":"Replaces
kqlFilter with searchConfiguration and uses Unified Search bar in APM
rules (#164540)\n\nFixes
https://github.com/elastic/kibana/issues/163781\r\n\r\n- `kqlFilter`
which was string is replaced with `searchConfiguration`\r\n- Replaces
KQL filter component with Unified Search bar\r\n- Removed \"Technical
Preview\" from the \"Filter\"
label","sha":"b88235aafe9b6993ef7bb3918f60c6d8462e9e46","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:APM","release_note:skip","backport:skip","auto-backport","Team:
Actionable
Observability","v8.10.0","v8.11.0"],"number":164540,"url":"https://github.com/elastic/kibana/pull/164540","mergeCommit":{"message":"Replaces
kqlFilter with searchConfiguration and uses Unified Search bar in APM
rules (#164540)\n\nFixes
https://github.com/elastic/kibana/issues/163781\r\n\r\n- `kqlFilter`
which was string is replaced with `searchConfiguration`\r\n- Replaces
KQL filter component with Unified Search bar\r\n- Removed \"Technical
Preview\" from the \"Filter\"
label","sha":"b88235aafe9b6993ef7bb3918f60c6d8462e9e46"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/164540","number":164540,"mergeCommit":{"message":"Replaces
kqlFilter with searchConfiguration and uses Unified Search bar in APM
rules (#164540)\n\nFixes
https://github.com/elastic/kibana/issues/163781\r\n\r\n- `kqlFilter`
which was string is replaced with `searchConfiguration`\r\n- Replaces
KQL filter component with Unified Search bar\r\n- Removed \"Technical
Preview\" from the \"Filter\"
label","sha":"b88235aafe9b6993ef7bb3918f60c6d8462e9e46"}}]}] BACKPORT-->

Co-authored-by: Bena Kansara <[email protected]>
@maryam-saeidi maryam-saeidi mentioned this pull request Aug 30, 2023
Closed
maryam-saeidi
maryam-saeidi reviewed Aug 30, 2023
View reviewed changes
x-pack/plugins/apm/public/components/alerting/ui_components/apm_rule_unified_search_bar.tsx

const { dataView } = useApmDataView();
const searchbarPlaceholder =
'Search for APM data… (e.g. service.name: service-1)';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we translate this?

This was referenced Oct 1, 2023
Merged
Merged
@jeanfabrice jeanfabrice mentioned this pull request Nov 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sorenlouv sorenlouv sorenlouv approved these changes

@maryam-saeidi maryam-saeidi maryam-saeidi left review comments

Assignees

@benakansara benakansara

Labels
auto-backport Deprecated - use backport:version if exact versions are needed backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v8.10.0 v8.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use Unified Search bar for the KQL filter in APM rules
7 participants
@benakansara @apmmachine @elasticmachine @kibana-ci @kibanamachine @sorenlouv @maryam-saeidi