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.

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

[Qradar] - add timeout param, update test-module and implement retry for connection errors #31339

Merged
merged 23 commits into from Dec 11, 2023

Conversation

GuyAfik
Copy link
Contributor

@GuyAfik GuyAfik commented Dec 6, 2023

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: link to the issue

Description

  • Added the timeout parameter to Qradar to allow querying http-requests for longer time.
  • Refactored the test-module to fetch only offenses.
  • Retry mechanism for http_request method in case of connection errors.

@GuyAfik GuyAfik changed the title Qradar add timeout param [Qradar] - add timeout param, update test-module and implement retry for connection errors Dec 6, 2023
@GuyAfik GuyAfik marked this pull request as ready for review December 11, 2023 09:25
@GuyAfik GuyAfik requested a review from ilaner as a code owner December 11, 2023 09:25
Copy link
Contributor

@ilaner ilaner left a comment

Choose a reason for hiding this comment

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

Very good job!

The reason we get the ReadTimeout and ConnectionErrors is because we have too many calls to QRadar.
Let's change the default for number of offenses to fetch in each fetch to 10, and add a configuration to configure the fetch interval parameter (it will just change the FETCH_SLEEP global)

Copy link
Contributor

@ilaner ilaner left a comment

Choose a reason for hiding this comment

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

Nice job!

Copy link
Contributor

@JudahSchwartz JudahSchwartz left a comment

Choose a reason for hiding this comment

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

lgtm!

@GuyAfik GuyAfik merged commit 7f7b4e9 into master Dec 11, 2023
16 of 17 checks passed
@GuyAfik GuyAfik deleted the qradar_add_timeout_param branch December 11, 2023 19:08
sapirshuker pushed a commit that referenced this pull request Dec 21, 2023
…for connection errors (#31339)

* add qradar timeout param

* add timeout to client

* add docs and bump rn

* fixes

* refactor test-module

* retry for http_request

* fixes

* elif

* rn and docker image

* rn

* bump rn

* update docker

* logger

* add fetch-interval and set default to 10

* README.md

* readme and udpates

* rn

* arg to number

* update docker-image

* ut

* fix uts
maimorag pushed a commit that referenced this pull request Dec 31, 2023
…for connection errors (#31339)

* add qradar timeout param

* add timeout to client

* add docs and bump rn

* fixes

* refactor test-module

* retry for http_request

* fixes

* elif

* rn and docker image

* rn

* bump rn

* update docker

* logger

* add fetch-interval and set default to 10

* README.md

* readme and udpates

* rn

* arg to number

* update docker-image

* ut

* fix uts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants