Skip to content

Conversation

@pyth0n1c
Copy link
Contributor

For integration testing, instead of scheduling a savedsearch to run and waiting a predefined amount of time, use the API to call the search right away. This can be a significant improvement in terms of the amount of time spent waiting for RBA/Notable artifacts to be generated.

@xqi-splunk xqi-splunk self-assigned this Jun 23, 2025
@xqi-splunk xqi-splunk marked this pull request as ready for review June 23, 2025 17:08
@xqi-splunk
Copy link
Collaborator

Code Changes

  • Use dispatch() to call the SavedSearch right away
  • Update the wait for risk/notable while loop and retry logic
    • Based on experiments, 45 detections need to retry dispatch() or run dispatch() after 50s to 120s after the savedsearch is enabled. Therefore, add a retry logic after 90s (50s is the corner case, most time needed falls under two intervals 70-80, and 120-130)
    • Based on experiments, some detections need extra waiting time for the risk/notable objects to be generated (like 20+ seconds). But since the validation process itself would need around 5s - 10s, we just let the validation process work as the natural wait time. Extra exponential wait time is added after 30s to avoid non necessary wait time. If wait time is added at the first place, regardless of the 30s, it would add 20 minutes extra test execution time.

Test Outcome

  • Decrease execute-testcases runtime from previous 111 minutes to 41 minutes.
  • Test result remain consistent, only Potential password in username failed. (No touch in the validation logic of risk/notable in this MR)

@xqi-splunk
Copy link
Collaborator

Code Changes

  • Update query for validate risk/notables to use latest job.sid after dispatch()

Test Outcome

  • Test pipeline execute-testcases runtime 45 minutes
  • Test result remain consistent, only Potential password in username failed.

Copy link
Contributor

@cmcginley-splunk cmcginley-splunk left a comment

Choose a reason for hiding this comment

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

Looks great Xiaonan! And the performance improvement is so so exciting; great job

Have a couple of structural changes requested, as well as sompoints for clarification

Copy link
Contributor

@cmcginley-splunk cmcginley-splunk left a comment

Choose a reason for hiding this comment

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

Looks great! A few more small tweaks and some points for conversation. @pyth0n1c please weigh in where appropriate :)

f'search index=risk search_name="{self.name}" [search index=risk search '
f'search_name="{self.name}" | tail 1 | fields orig_sid] | tojson'
)
if self.sid is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

For option 2, I was only suggesting that bool field in the case we still wanted to do cleanup pre-test. But I agree with Eric for all the reasons mentioned. I would remove this code path (or throw on self.sid is None) and remove the pre-test cleanup

f'search index=notable search_name="{self.name}" [search index=notable search '
f'search_name="{self.name}" | tail 1 | fields orig_sid] | tojson'
)
if self.sid is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment above

Copy link
Contributor

@cmcginley-splunk cmcginley-splunk left a comment

Choose a reason for hiding this comment

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

Looks great xiaonan :) approved

@cmcginley-splunk cmcginley-splunk merged commit 6df8352 into main Jul 9, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants