feat(investigation): Add eventTypes filter on the API #202829

kdelemme · 2024-12-03T22:12:26Z

Resolves #200800

🏇🏻 Summary

Note

I've implemented a basic UI selector using the existing EuiFilterGroup instead of the Figma UI that requires a more custom component. We can always iterate later.

This PR adds an optional eventTypes filter on the observability events API. The eventTypes string is checked against the event type enum, and transformed to an array for easier usage within the boundary of the server.
When no specified, all event types are returned.

🍰 Boy/Girl scouts rule: I've also refactored/cleaned a few shenanigans I've introduced a while back in the Investigation context instead of leveraging react query context directly.

Screenshots

rca_filter_events.mov

…fix'

elasticmachine · 2024-12-04T20:51:47Z

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

github-actions · 2024-12-04T21:25:50Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

fkanout · 2024-12-11T10:17:03Z

packages/kbn-investigation-shared/src/rest_specs/get_events.ts

@@ -8,7 +8,7 @@
 */

 import { z } from '@kbn/zod';


What is this none self-explanatory z 🤔 😆 ?

Did I add that? I don't recall importing from @kbn/zod but that's probably just a wrapper around zod itself

https://zod.dev/?id=introduction

I didn't know it before. Thanks!

fkanout

I'm not sure if it's related to the PR OR if this could be fixed in it, but the page is not fully responsive.

fkanout · 2024-12-11T11:37:21Z

...solution/investigate_app/server/routes/get_global_investigate_app_server_route_repository.ts

+
+    if (
+      annotationsClient &&
+      (includeAllEventTypes || params?.query?.eventTypes?.includes('annotation'))


I wonder how robust is this check params?.query?.eventTypes?.includes('annotation'))? Do we have another way to check this?

Zod ensures that if eventTypes is present on the query parameters, then it is an array of event type (enum).

I could write the same check as params && params.query && params.query.eventTypes && params.queryEventTypes.includes('annotation') but I would be reinventing optional chaining. I don't think it's more readable this way, but it's definitely more verbose 😅

@fkanout Do you want me to address this somehow?

@kdelemme thank you for explaining it; it's more clear now. My only concern is the hard-coded string annotation type. If it's an enum, I would expect to use it instead.

Ah yes good point. I forgot to parse the eventTypes: ac24d91

fkanout · 2024-12-11T11:39:14Z

...ails/components/investigation_timeline/investigation_search_bar/investigation_search_bar.tsx

+      alignItems="flexStart"
+      justifyContent="spaceBetween"
+      responsive
+      css={css`


Out of curiosity, why do we use the css emotion in this inline style instead of the built-in style?

I tend to use it all the time, just in case I need to write nested selectors. But true it is not needed here, and style would work the same

kdelemme · 2024-12-11T13:19:29Z

Going to check the responsiveness, good catch

kdelemme · 2024-12-11T18:38:44Z

@fkanout The responsiveness is already broken on main, I'm going to open a few tickets to keep track of these things

kdelemme · 2024-12-11T18:40:18Z

...vestigation_timeline/investigation_timeline_filter_bar/investigation_timeline_filter_bar.tsx

+  onEventTypesSelected: (eventTypes: string[]) => void;
+}
+
+export function InvestigationTimelineFilterBar({ onEventTypesSelected }: Props) {


💬 Simplified (and renamed) the timeline filter bar: replaced uiSearch with dateRange eui component and moved the update function into here.

kdelemme · 2024-12-11T18:40:47Z

...nvestigation_timeline/investigation_timeline_filter_bar/investigation_event_types_filter.tsx

+  onSelected: (eventTypes: string[]) => void;
+}
+
+export function InvestigationEventTypesFilter({ onSelected }: Props) {


💬 The event filter dropdown component

fkanout

LGTM. 💪🏻
@kdelemme leave it to you how to handle the annotation enum if you think there is a better way. And to solve the merge conficlts.

elasticmachine · 2024-12-12T16:03:59Z

⏳ Build in-progress

Buildkite Build
Commit: cf33ccc
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-202829-cf33ccc6a858

History

💔 Build #259799 failed ac24d91
💛 Build #259425 was flaky a7043e0
💚 Build #259389 succeeded 49ac988
💚 Build #259175 succeeded f20d1ca
💚 Build #257129 succeeded 86c7081

cc @kdelemme

kibanamachine · 2024-12-12T16:49:52Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12300981413

(cherry picked from commit 2ab38a3)

kibanamachine · 2024-12-12T16:54:48Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

…#204089) # Backport This will backport the following commits from `main` to `8.x`: - [feat(investigation): Add eventTypes filter on the API (#202829)](#202829)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Kevin Delemme <kevin.delemme@elastic.co>

kdelemme and others added 12 commits December 3, 2024 13:59

Remove adding note from context

eb0fd06

Remove the need for abstraction in context

7a3cec5

Remove cacheTime and some staleTime

09c01b7

Remove extra div

2ee63e4

Add types query parameter in events API

eda1e2e

Export event type

09dabb2

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

8af1f94

…fix'

Add filter

dc71bc8

Create timeline main component

66b3178

Move into components

e771b82

Move into components

48ad2fa

Remove loading placeholder

6cff219

kdelemme force-pushed the investigate/add-filter branch from b2b96d0 to 6cff219 Compare December 4, 2024 20:43

Merge branch 'main' into investigate/add-filter

0be2277

kdelemme added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:obs-ux-management Observability Management User Experience Team v8.18.0 labels Dec 4, 2024

kdelemme self-assigned this Dec 4, 2024

kdelemme marked this pull request as ready for review December 4, 2024 20:51

kdelemme requested a review from a team as a code owner December 4, 2024 20:51

Fix type

86c7081

botelastic bot added the ci:project-deploy-observability Create an Observability project label Dec 4, 2024

fkanout self-requested a review December 11, 2024 09:59

Merge branch 'main' into investigate/add-filter

f20d1ca

fkanout reviewed Dec 11, 2024

View reviewed changes

kdelemme added 2 commits December 11, 2024 10:39

Replace searchbar with daterange

49ac988

Some cleanup

a7043e0

kdelemme commented Dec 11, 2024

View reviewed changes

fkanout approved these changes Dec 12, 2024

View reviewed changes

kdelemme added 2 commits December 12, 2024 08:28

Infer type when parsed

ac24d91

Merge branch 'main' into investigate/add-filter

cf33ccc

kdelemme enabled auto-merge (squash) December 12, 2024 15:00

kdelemme merged commit 2ab38a3 into elastic:main Dec 12, 2024
8 checks passed

kibanamachine added the v9.0.0 label Dec 12, 2024

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 12, 2024

feat(investigation): Add eventTypes filter on the API (elastic#202829)

592d8d2

(cherry picked from commit 2ab38a3)

kibanamachine mentioned this pull request Dec 12, 2024

[8.x] feat(investigation): Add eventTypes filter on the API (#202829) #204089

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(investigation): Add eventTypes filter on the API #202829

feat(investigation): Add eventTypes filter on the API #202829

kdelemme commented Dec 3, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Dec 4, 2024

github-actions bot commented Dec 4, 2024

fkanout Dec 11, 2024

kdelemme Dec 11, 2024

kdelemme Dec 11, 2024

fkanout Dec 12, 2024

fkanout left a comment

fkanout Dec 11, 2024

kdelemme Dec 11, 2024

kdelemme Dec 11, 2024

fkanout Dec 12, 2024

kdelemme Dec 12, 2024

fkanout Dec 11, 2024

kdelemme Dec 11, 2024

kdelemme commented Dec 11, 2024

kdelemme commented Dec 11, 2024

kdelemme Dec 11, 2024

kdelemme Dec 11, 2024

fkanout left a comment •

edited

Loading

elasticmachine commented Dec 12, 2024

kibanamachine commented Dec 12, 2024

kibanamachine commented Dec 12, 2024

feat(investigation): Add eventTypes filter on the API #202829

feat(investigation): Add eventTypes filter on the API #202829

Conversation

kdelemme commented Dec 3, 2024 • edited by kibanamachine Loading

🏇🏻 Summary

elasticmachine commented Dec 4, 2024

github-actions bot commented Dec 4, 2024

🤖 GitHub comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fkanout left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdelemme commented Dec 11, 2024

kdelemme commented Dec 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fkanout left a comment • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Dec 12, 2024

⏳ Build in-progress

History

kibanamachine commented Dec 12, 2024

kibanamachine commented Dec 12, 2024

💚 All backports created successfully

Questions ?

kdelemme commented Dec 3, 2024 •

edited by kibanamachine

Loading

fkanout left a comment •

edited

Loading