Skip to content

Nbensalmon/ciac 13043/collection extrahop reveal x #39545

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

Open
wants to merge 51 commits into
base: master
Choose a base branch
from

Conversation

nbensalm-palo
Copy link
Contributor

Contributing to Cortex XSOAR Content

Make sure to register your contribution by filling the contribution registration form

The Pull Request will be reviewed only after the contribution registration form is filled.

Status

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

Related Issues

fixes: link to the issue

@nbensalm-palo nbensalm-palo requested a review from Shellyber April 10, 2025 10:54
Copy link

Your contributed ExtraHop pack has been modified on files:

Packs/ExtraHop/pack_metadata.json
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/ExtrahopRevealXEventCollector.py
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/ExtrahopRevealXEventCollector_image.png
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/command_examples.txt
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/test_data/detections-dummy.json
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/ExtrahopRevealXEventCollector.yml
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/ExtrahopRevealXEventCollector_test.py
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/README.md
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector/ExtrahopRevealXEventCollector_description.md
Please review the changes here

Copy link

github-actions bot commented Apr 10, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
Packs/ExtraHop/Integrations/ExtrahopRevealXEventCollector
   ExtrahopRevealXEventCollector.py1627653%45, 59–62, 65–66, 69–70, 74, 76, 89, 91, 95, 100, 106–107, 109, 115–116, 131–134, 144, 146, 161, 165, 190, 243, 279, 286–287, 310, 314–315, 348–349, 352, 357–358, 360, 365, 370–372, 374–381, 383, 390, 392–408, 410, 412–413
TOTAL1627653% 

Tests Skipped Failures Errors Time
6 0 💤 0 ❌ 0 🔥 2.891s ⏱️

@Dan-at-Extrahop
Copy link
Contributor

Dan-at-Extrahop commented Apr 10, 2025

@nbensalm-palo can you please provide me with some background on these additions? I'm curious how this new integration relates to the existing ExtraHop_v2 Integration, which already includes a fetch-incidents command.

Trying to understand if it makes more sense to add new features to the existing integration instead of creating a new separate integration.

Also I'm wondering if this is an internal contribution from Palo Alto Networks or a community contribution?

Thanks

Copy link
Contributor

@Shellyber Shellyber left a comment

Choose a reason for hiding this comment

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

Nice work

@Shellyber
Copy link
Contributor

@ShirleyDenkberg please have a look when you got the chance.

@Shellyber
Copy link
Contributor

Hi @Dan-at-Extrahop,
First, this is an internal contribution developed and supported by our content team.

Second, while we’re aware that the main integration you contributed includes a fetch_incidents function, the XSIAM platform requires a different approach for handling detections.
To support this, we follow a best practice of implementing a specialized integration type known as an event collector, which is tailored specifically for the XSIAM platform.

@Dan-at-Extrahop
Copy link
Contributor

Hi @Dan-at-Extrahop, First, this is an internal contribution developed and supported by our content team.

Second, while we’re aware that the main integration you contributed includes a fetch_incidents function, the XSIAM platform requires a different approach for handling detections. To support this, we follow a best practice of implementing a specialized integration type known as an event collector, which is tailored specifically for the XSIAM platform.

Hi @Shellyber,

Thanks for the response and clarifications, that addresses both of my concerns. I appreciate the work that your teams continue to do to contribute and support these integrations. Nice work.

@content-bot
Copy link
Collaborator

⚠️ The PR is missing the ready-for-pipeline-running label. Please add the label when the PR is ready in order to proceed.

@content-bot
Copy link
Collaborator

Validate summary

Verdict: PR can be force merged from validate perspective? ✅

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.

5 participants