Skip to content

[Enhancement] Modsecurity event coverage #4521

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Nov 1, 2022
Merged

[Enhancement] Modsecurity event coverage #4521

merged 11 commits into from
Nov 1, 2022

Conversation

WildDogOne
Copy link
Contributor

@WildDogOne WildDogOne commented Oct 28, 2022 •
edited
Loading

What does this PR do?

This PR is aimed at getting better coverage on Modsecurity logs.
Mostly focused on the http request side (response is already covered well)
also some bugfix with missing fields ;)

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.

@WildDogOne WildDogOne requested a review from a team as a code owner October 28, 2022 18:26
@elasticmachine
Copy link

elasticmachine commented Oct 28, 2022 •
edited
Loading

💚 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-10-31T07:35:56.510+0000

  • Duration: 15 min 57 sec

Test stats 🧪

Test Results
Failed 0
Passed 6
Skipped 0
Total 6

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@efd6
Copy link
Contributor

efd6 commented Oct 30, 2022

/test

@elasticmachine
Copy link

elasticmachine commented Oct 30, 2022 •
edited
Loading

🚀 Benchmarks report

Package modsecurity 👍(0) 💚(0) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
auditlog 1223.99 897.67 -326.32 (-26.66%) 💔

To see the full report comment with /test benchmark fullreport

@elasticmachine
Copy link

elasticmachine commented Oct 30, 2022 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (3/3) 💚 2.749
Classes 100.0% (3/3) 💚 2.749
Methods 100.0% (28/28) 💚 10.058
Lines 95.088% (271/285) 👍 3.585
Conditionals 100.0% (0/0) 💚

efd6
efd6 reviewed Oct 30, 2022
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

I'm wondering whether it would be valuable to have modsecurity.request.headers and modsecurity.response.headers both as flattened. This could be guarded by an advanced option to prevent overloading indices.

@WildDogOne @efd6
Update packages/modsecurity/changelog.yml
4f62a98 
Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
@WildDogOne
Copy link
Contributor Author

@efd6 I have no actual opinion on this xD
I know I did have some issues with flattened field types in the past, but that was mostly due to changes in the mapping associated to this

@efd6
Copy link
Contributor

efd6 commented Oct 30, 2022

/test

@efd6
Copy link
Contributor

efd6 commented Oct 30, 2022

It is always something that we can add in the future if it becomes needed.

@efd6
Copy link
Contributor

efd6 commented Oct 31, 2022

/test

efd6
efd6 approved these changes Nov 1, 2022
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

Thanks

@efd6 efd6 merged commit c663da0 into elastic:main Nov 1, 2022
@WildDogOne WildDogOne deleted the modsecurity-event-coverage branch November 1, 2022 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efd6 efd6 efd6 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@WildDogOne @elasticmachine @efd6