Skip to content

[crowdstrike.fdr] Use 'params' for data table in script processors - #3492

Merged
andrewkroh merged 8 commits into
elastic:mainfrom
andrewkroh:crowdstrike/fdr/reuse-params
Jun 9, 2022
Merged

andrewkroh merged 8 commits into
elastic:mainfrom
andrewkroh:crowdstrike/fdr/reuse-params

Conversation

@andrewkroh

@andrewkroh andrewkroh commented Jun 8, 2022 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Prior to this change the data type (HashMap) was constructed on each script processor iteration. By using params the same map is reused, but care must be taken to ensure that the params data is not modified. That involves making copies of any mutable lists.

Based on local testing this changes the avg execution time of the categorize-events script processor the from 71,092 ns to 5,810 ns.

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.

Screenshots

This was done from a containerized ES running on my local machine so don't read into the overall numbers. The relative difference in the categorize-events script is the important part.

Before:

Screen Shot 2022-06-08 at 10 47 39

After:

Screen Shot 2022-06-08 at 17 11 40

In my local testing this changes the avg execution time of the categorize-events script processor the from 71,092 ns to 5,810 ns.

When the map is built inside the script it must be created each time. By using params the same map is reused, but care
must be taken to ensure that the params data is modified. That involves making copies of any mutable lists.

Remove script
Don't use null-safe operation when ctx can never be null
Avoid the need add escapes.
There are no sub-pipelines so remove that part of the message.
@andrewkroh
andrewkroh marked this pull request as ready for review June 8, 2022 21:32
@andrewkroh
andrewkroh requested a review from a team as a code owner June 8, 2022 21:32
@elasticmachine

Copy link
Copy Markdown

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine

Copy link
Copy Markdown

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-06-08T21:27:40.629+0000

  • Duration: 16 min 36 sec

Test stats 🧪

Test Results
Failed 0
Passed 13
Skipped 0
Total 13

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine

Copy link
Copy Markdown

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 100.0% (9/9) 💚 3.432
Classes 100.0% (9/9) 💚 3.432
Methods 100.0% (53/53) 💚 11.111
Lines 95.047% (2610/2746) 👍 5.29
Conditionals 100.0% (0/0) 💚

@andrewkroh
andrewkroh merged commit e926d4e into elastic:main Jun 9, 2022
orestisfl pushed a commit to orestisfl/integrations that referenced this pull request May 15, 2026
…lastic#3492)

Prior to this change the data type (HashMap) was constructed on each script processor iteration. By using params the same map is reused, but care must be taken to ensure that the params data is not modified. That involves making copies of any mutable lists.

Based on local testing this changes the avg execution time of the categorize-events script processor the from 71,092 ns to 5,810 ns.

* Replace ctx?. with ctx.

Don't use null-safe operation when ctx can never be null

* Use triple-brace in mustache templates

Avoid the need to add escapes.

* Add tag to all script processors.

* Use params for dns request type

* Adjust error message

There are no sub-pipelines so remove that part of the message.

* Reduce high failure count on 5_date_date-context-timestamp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Integration:crowdstrike CrowdStrike

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants