-
Notifications
You must be signed in to change notification settings - Fork 387
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
pulse_connect_secure: add syslog priority format parsing support #2552
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
@@ -23,6 +23,7 @@ processors: | |||
field: event.original | |||
patterns: | |||
- '%{SYSLOGTIMESTAMP} %{SYSLOGHOST:host.hostname} %{INT} %{TIMESTAMP_ISO8601:_tmp.timestamp} %{IP:observer.ip} PulseSecure: - - - %{DATE2} - %{SYSLOGHOST:observer.name} - \[%{IPORHOST:client.address}\] %{USERNAME:user.name}?\(%{DATA:pulse_secure.realm}?\)\[%{DATA:pulse_secure.role}\] - %{GREEDYDATA:message}' | |||
- '^<%{NONNEGINT}>%{NUMBER}? %{TIMESTAMP_ISO8601:_tmp.timestamp} %{IP:observer.ip} PulseSecure: - - - %{DATE2} - %{SYSLOGHOST:observer.name} - \[%{IPORHOST:client.address}\] %{USERNAME:user.name}?\(%{DATA:pulse_secure.realm}?\)\[%{DATA:pulse_secure.role}\] - %{GREEDYDATA:message}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would combine the 2 patterns and just add the <%{NONNEGINT}>%{NUMBER}?
as a optional item. Besides that, the pattern seems the same thought u changed the host.*
to observer.*
which might have been the right thing to begin with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Yes, it is identical beyond the prefix (no change to host.*
. The issue with just making the PRI optional is that the %{SYSLOGTIMESTAMP} %{SYSLOGHOST:host.hostname} %{INT}
prefix components do not appear in test cases that prompted the change, so they need to be made optional too, and then an order for the case that they both appear has to be decided. What would you suggest there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't spot that difference, THen the best thing i'd do is consolidate the similar sections into sub grok patterns as much as you can so its easier to maintain long term
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL
@@ -22,7 +22,7 @@ processors: | |||
- grok: | |||
field: event.original | |||
patterns: | |||
- '%{SYSLOGTIMESTAMP} %{SYSLOGHOST:host.hostname} %{INT} %{TIMESTAMP_ISO8601:_tmp.timestamp} %{IP:observer.ip} PulseSecure: - - - %{DATE2} - %{SYSLOGHOST:observer.name} - \[%{IPORHOST:client.address}\] %{USERNAME:user.name}?\(%{DATA:pulse_secure.realm}?\)\[%{DATA:pulse_secure.role}\] - %{GREEDYDATA:message}' | |||
- '^(<%{NONNEGINT}>%{NUMBER}?|%{SYSLOGTIMESTAMP} %{SYSLOGHOST:host.hostname} %{INT}) %{TIMESTAMP_ISO8601:_tmp.timestamp} %{IP:observer.ip} PulseSecure: - - - %{DATE2} - %{SYSLOGHOST:observer.name} - \[%{IPORHOST:client.address}\] %{USERNAME:user.name}?\(%{DATA:pulse_secure.realm}?\)\[%{DATA:pulse_secure.role}\] - %{GREEDYDATA:message}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about NONNEGINT:log.syslog.priority:long
to store the value?
https://www.elastic.co/guide/en/ecs/current/ecs-log.html#field-log-syslog-priority
…stic#2552) * remove event.ingested * pulse_connect_secure: add syslog priority format parsing support
What does this PR do?
This adds support for syslog priority value parsing.
Checklist
changelog.yml
file.Author's Checklist
First commit is mechanical; this just runs the new version of elastic-package on to reduce diff noise in commit two.
How to test this PR locally
elastic-package test
in package/pulse_connect_secureRelated issues
Screenshots
N/A