[RHCLOUD-36086] Refactor Splunk integration actions to not use behavior group apis anymore #627

g-duval · 2024-11-05T15:16:21Z

Description

Since we are moving away from Notifications behavior groups, Splunk integration have to be updated.
It's now possible create an integration and to link it to sevral event types of sevral bundles with a single request.

RHCLOUD-36086

Checklist ☑️

PR only fixes one issue or story
Change reviewed for extraneous code
UI best practices adhered to
Commits squashed and meaningfully named
All PR checks pass locally (build, lint, test, E2E)

(Optional) QE: Needs QE attention (OUIA changed, perceived impact to tests, no test coverage)
(Optional) QE: Has been mentioned
(Optional) UX: Needs UX attention (end user UX modified, missing designs)
(Optional) UX: Has been mentioned

codecov-commenter · 2024-11-05T15:23:48Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.00%. Comparing base (ce1febd) to head (08c1ddf).
Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #627   +/-   ##
=======================================
  Coverage   90.00%   90.00%           
=======================================
  Files           3        3           
  Lines          20       20           
=======================================
  Hits           18       18           
  Misses          2        2

g-duval · 2024-11-15T15:54:44Z

@vkrizan Could you please have a look to this PR?

vkrizan

Thank you @g-duval. There is one breaking change with the bundle events not being limited. See below.

vkrizan · 2024-11-18T13:27:34Z

src/pages/Integrations/SplunkSetup/useSplunkSetup.tsx

@@ -33,26 +33,15 @@ Steps:
 ["ff59b502-da25-4297-bd88-6934ad0e0d63"] <<- Behavior group ID


Can you please remove the comment (or update it)?

Sure, and sorry I forgot to update comments

vkrizan · 2024-11-18T13:30:01Z

src/pages/Integrations/SplunkSetup/useSplunkSetup.tsx

-  BehaviorGroupRequest,
-  UUID,
-} from '../../../types/Notification';
+import { UUID } from '../../../types/Notification';

 export const SPLUNK_GROUP_NAME = 'SPLUNK_INTEGRATION';
 export const SPLUNK_INTEGRATION_NAME = 'SPLUNK_AUTOMATION';


Please remove the SPLUNK_BEHAVIOR_GROUP_NAME constant.

vkrizan · 2024-11-18T13:41:42Z

src/pages/Integrations/SplunkSetup/useSplunkSetup.tsx

    const eventTypes = await getAllEventTypes();

    const selectedEventTypes = eventTypes.filter((eventType) => {
      if (!eventType?.application) {
        return false;
      }

-      if (eventType.application.bundle_id !== behaviorGroup.bundleId) {
-        return false;


This part limited the event types to be part of a single bundle (RHEL/Insights). With this removal, this would attach events that match the name across all bundles. For example, Ansible has events not only for RHEL but also for OpenShift. Please update the event types selection to filter the bundle.

That's right, thank you, I updated it.

vkrizan · 2024-11-18T13:42:39Z

src/pages/Integrations/SplunkSetup/useSplunkSetup.tsx

-        onProgress(' ERROR!\n', 'pf-v5-u-danger-color-200');
-        console.log(error);
-      }
+      onProgress(' FETCHED\n', 'pf-v5-u-success-color-200');


We can move this after the for-loop, as there is no async call in-between.

…or group apis anymore

karelhala · 2024-11-20T16:06:16Z

@g-duval thats awesome thinking ahead! I wanted to create a JIRA and tackle this within our team next quarter. I'd probably wait a bit and maybe sync on it more. Right now the new integrations flow is in development and we are missing 2 or 3 features from full feature rediness.

Or is this just updating current splunk flow? We will probably revisit the new wizard with option to include splunk in it with UX as well.

vkrizan · 2024-11-20T17:28:05Z

src/pages/Integrations/SplunkSetup/useSplunkSetup.tsx


-    await attachEvents(behaviorGroup, events, onProgress);
+  const getbundleTabs = () => {
+    if (getBundles.payload?.status === 200) {


Isn't the getBundles an async function that needs to be called?

Well I'm not sure, it is not used as async in other places of this project, so I used the same way to use it to be consistent.

Is this the imported function?:

notifications-frontend/src/services/Notifications/GetBundles.ts

Lines 10 to 13 in 23f50fc

export const useGetBundles = (

includeApplications?: boolean,

initFetch = true

) => useQuery(getBundlesAction(!!includeApplications), initFetch);

Can you please double check?

I feel like there is an incorrect wiring of these hooks. The ones that are created in this module do call async functions, and are hooks due to the frameworks that they take request capabilities from. The useGetBundle would create a hook useQuery of react-fetching-library. The useQuery is set to issue the request immediately when the hook is created, triggering a rerender. Should the async function of useSplunkSetup was called immedeately or before the useGetBundles finishes loading, it would fail with the "error fetching bundle ...".

I recommend to set initFetch = false and then use query variable to manually call async request at the right time. See https://marcin-piela.github.io/react-fetching-library/#/?id=usequery

Ok thank you, I updated this part, could you please have a look? BTW I also used an existing method to retrieve only one bundle.

Thank you. It looks good. One minor thing, you might bring back a onProgress() before calling the async getBundle stuff to announce that step to the user.

g-duval · 2024-11-25T08:55:03Z

@g-duval thats awesome thinking ahead! I wanted to create a JIRA and tackle this within our team next quarter. I'd probably wait a bit and maybe sync on it more. Right now the new integrations flow is in development and we are missing 2 or 3 features from full feature rediness.

Or is this just updating current splunk flow? We will probably revisit the new wizard with option to include splunk in it with UX as well.

Hi @karelhala , yes the idea is to update only the current Splunk integration to move away from behavior group API as soon as possible, because we don't want to implement behavior groups model in Kessel for couple of month only.
Of course, you could revisit this screen later in 2025 is needed.

vkrizan

Looks good.
If possible, please test it out before merging.

Thank you!

karelhala · 2024-12-09T13:05:43Z

/retest

g-duval · 2024-12-11T09:29:06Z

Yes I developed and tested it using stage backend, but I will definitely test it again when it will be deployed in stage.

g-duval force-pushed the RHCLOUD-36086_refactorSplunkIntegrationActionsWithoutUsingBGs branch from c7a3110 to 1430821 Compare November 7, 2024 13:54

g-duval marked this pull request as ready for review November 12, 2024 10:08

vkrizan reviewed Nov 18, 2024

View reviewed changes

g-duval added 2 commits November 20, 2024 14:47

[RHCLOUD-36086] Refactor Splunk integration actions to not use behavi…

def5158

…or group apis anymore

Fixes from code review

20a7d2e

g-duval force-pushed the RHCLOUD-36086_refactorSplunkIntegrationActionsWithoutUsingBGs branch from 3c0f374 to 20a7d2e Compare November 20, 2024 13:48

g-duval requested a review from a team as a code owner November 20, 2024 13:48

vkrizan reviewed Nov 20, 2024

View reviewed changes

Updates from code review

08c1ddf

vkrizan approved these changes Nov 26, 2024

View reviewed changes

karelhala approved these changes Dec 9, 2024

View reviewed changes

fhlavac approved these changes Dec 11, 2024

View reviewed changes

g-duval merged commit c516efc into RedHatInsights:master Dec 11, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RHCLOUD-36086] Refactor Splunk integration actions to not use behavior group apis anymore #627

[RHCLOUD-36086] Refactor Splunk integration actions to not use behavior group apis anymore #627

g-duval commented Nov 5, 2024

codecov-commenter commented Nov 5, 2024 •

edited

Loading

g-duval commented Nov 15, 2024

vkrizan left a comment

vkrizan Nov 18, 2024

g-duval Nov 20, 2024

vkrizan Nov 18, 2024

g-duval Nov 20, 2024

vkrizan Nov 18, 2024

g-duval Nov 20, 2024

vkrizan Nov 18, 2024

karelhala commented Nov 20, 2024

vkrizan Nov 20, 2024

g-duval Nov 25, 2024

vkrizan Nov 25, 2024 •

edited

Loading

vkrizan Nov 25, 2024 •

edited

Loading

g-duval Nov 26, 2024

vkrizan Nov 26, 2024

g-duval commented Nov 25, 2024

vkrizan left a comment

karelhala commented Dec 9, 2024

g-duval commented Dec 11, 2024

		@@ -33,26 +33,15 @@ Steps:
		["ff59b502-da25-4297-bd88-6934ad0e0d63"] <<- Behavior group ID

	export const useGetBundles = (
	includeApplications?: boolean,
	initFetch = true
	) => useQuery(getBundlesAction(!!includeApplications), initFetch);

[RHCLOUD-36086] Refactor Splunk integration actions to not use behavior group apis anymore #627

[RHCLOUD-36086] Refactor Splunk integration actions to not use behavior group apis anymore #627

Conversation

g-duval commented Nov 5, 2024

Description

Checklist ☑️

codecov-commenter commented Nov 5, 2024 • edited Loading

Codecov Report

g-duval commented Nov 15, 2024

vkrizan 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

karelhala commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkrizan Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

vkrizan Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g-duval commented Nov 25, 2024

vkrizan left a comment

Choose a reason for hiding this comment

karelhala commented Dec 9, 2024

g-duval commented Dec 11, 2024

codecov-commenter commented Nov 5, 2024 •

edited

Loading

vkrizan Nov 25, 2024 •

edited

Loading

vkrizan Nov 25, 2024 •

edited

Loading