Skip to content

carbon_black_cloud: ensure alert search range is a valid temporal ordering - #11149

Merged
efd6 merged 2 commits into
elastic:mainfrom
efd6:s5128-carbon_black_cloud
Sep 19, 2024
Merged

carbon_black_cloud: ensure alert search range is a valid temporal ordering#11149
efd6 merged 2 commits into
elastic:mainfrom
efd6:s5128-carbon_black_cloud

Conversation

@efd6

@efd6 efd6 commented Sep 17, 2024

Copy link
Copy Markdown
Contributor

Proposed commit message

See title.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots
@efd6 efd6 added Integration:carbon_black_cloud VMware Carbon Black Cloud bugfix Pull request that fixes a bug issue Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] labels Sep 17, 2024
@efd6 efd6 self-assigned this Sep 17, 2024
@efd6
efd6 force-pushed the s5128-carbon_black_cloud branch from 934b3e0 to d3cbf7f Compare September 17, 2024 00:03
@efd6 efd6 changed the title carbon_black_cloud: nsure alert search range is a valid temporal ordeing Sep 17, 2024
@efd6
efd6 force-pushed the s5128-carbon_black_cloud branch from d3cbf7f to 336e08f Compare September 17, 2024 00:14
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Sep 17, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

🚀 Benchmarks report

Package carbon_black_cloud 👍(2) 💚(3) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
asset_vulnerability_summary 11494.25 9259.26 -2234.99 (-19.44%) 💔

To see the full report comment with /test benchmark fullreport
@efd6
efd6 marked this pull request as ready for review September 17, 2024 00:44
@efd6
efd6 requested a review from a team as a code owner September 17, 2024 00:44
@elasticmachine

elasticmachine commented Sep 17, 2024

Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)
chrisberkhout
chrisberkhout requested changes Sep 17, 2024
View reviewed changes
Comment thread packages/carbon_black_cloud/data_stream/alert_v7/agent/stream/cel.yml.hbs Outdated
// an event that is after this delay for the next evaluation. This
// would result in an inverted time range. So clamp the range start
// to the last event we received.
"time_range": (now - duration("15m")).as(delayed, {

@chrisberkhout chrisberkhout Sep 17, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This logic looks good.

As mentioned elsewhere, I think we can reduce the 15m to 60s, remove the note in the README and link to the documentation here.

@efd6 efd6 Sep 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you think it's worth making this configurable?

@chrisberkhout chrisberkhout Sep 18, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, I don't think so.

I think it's probably just a matter of a timestamp being assigned a value in application code and then written after a variable delay, or being assigned in a database with multiple write nodes and then replicated with variable delay. So they're out of order by roughly the duration of a request. There could be other systems with longer delays, but if they say 60 seconds it's probably 1ms - 15s that we're really worried about.
@elasticmachine

elasticmachine commented Sep 17, 2024

Copy link
Copy Markdown

💚 Build Succeeded

History

cc @efd6
@elastic-sonarqube

elastic-sonarqube Bot commented Sep 17, 2024

Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube
@efd6
efd6 requested a review from chrisberkhout September 18, 2024 08:39
@efd6 efd6 changed the title carbon_black_cloud: ensure alert search range is a valid temporal ordeing Sep 18, 2024
@efd6
efd6 merged commit 6ecaa71 into elastic:main Sep 19, 2024
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Sep 19, 2024

Copy link
Copy Markdown
Contributor

Package carbon_black_cloud - 2.5.2 containing this change is available at https://epr.elastic.co/search?package=carbon_black_cloud
@jamiehynds

jamiehynds commented Sep 23, 2024

Copy link
Copy Markdown

@efd6 there's a custom on our Discuss forums still reporting an issue with Carbon Black after updating to the latest integration. https://discuss.elastic.co/t/carbon-black-cloud-cel-alert-v7-400-bad-request/366464/3
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
@efd6
carbon_black_cloud: ensure alert search range is a valid temporal ord…
a4f9716 
…ering (elastic#11149)
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
@efd6
carbon_black_cloud: ensure alert search range is a valid temporal ord…
152153d 
…ering (elastic#11149)
@efd6
efd6 deleted the s5128-carbon_black_cloud branch February 5, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes a bug issue Integration:carbon_black_cloud VMware Carbon Black Cloud Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

4 participants

@efd6 @elasticmachine @jamiehynds @chrisberkhout