system/data_stream/syslog: Message Parsing issue and exclusion of files in syslog datastream by ishleenk17 · Pull Request #8621 · elastic/integrations

ishleenk17 · 2023-11-30T17:49:27Z

Proposed commit message

There are 2 changes as part of the PR.
1. Making the exclude_files configurable for the user asd user might want to exclude different type of files.
2. Resolving the message parsing issue in Syslog

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.

Related issues

Relates https://github.com/elastic/sdh-beats/issues/4056, https://github.com/elastic/sdh-beats/issues/3596

elasticmachine · 2023-11-30T18:09:25Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-12-01T08:57:03.770+0000
Duration: 18 min 29 sec

Test stats 🧪

Test	Results
Failed	0
Passed	158
Skipped	0
Total	158

🤖 GitHub comments

Expand to view the GitHub comments

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

/test : Re-trigger the build.

elasticmachine · 2023-11-30T18:09:37Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`3/3`)	💚
Files	100.0% (`4/4`)	💚 15.385
Classes	100.0% (`4/4`)	💚 15.385
Methods	67.857% (`57/84`)	👎 -21.973
Lines	98.426% (`3002/3050`)	👍 9.212
Conditionals	100.0% (`0/0`)	💚

devamanv · 2023-11-30T18:41:54Z

@ishleenk17 Quick question: if we are trying to make sure that the message field doesn't already exist, could we just check for message == null in the second rename processor(if a check like that is possible), instead of adding a new remove processor?
Rest looks good to me

shmsr · 2023-11-30T19:12:11Z

@ishleenk17 Quick question: if we are trying to make sure that the message field doesn't already exist, could we just check for message == null in the second rename processor(if a check like that is possible), instead of adding a new remove processor? Rest looks good to me

++

In the rename processor, if we add the on_failure i.e., it'd fail if event.original != null? So, with on_failure we can pass the remove processor with no condition. Is this gonna work? If yes, removal would happen in fewer processing.

efd6

In looking for context, I can see that the literal .gz$ is used everywhere for this pattern. Probably there should be an issue for this.

ishleenk17 · 2023-12-01T04:58:50Z

@ishleenk17 Quick question: if we are trying to make sure that the message field doesn't already exist, could we just check for message == null in the second rename processor(if a check like that is possible), instead of adding a new remove processor? Rest looks good to me

That won;t suffice as we want to remove the message field

ishleenk17 · 2023-12-01T05:08:23Z

@ishleenk17 Quick question: if we are trying to make sure that the message field doesn't already exist, could we just check for message == null in the second rename processor(if a check like that is possible), instead of adding a new remove processor? Rest looks good to me

++

In the rename processor, if we add the on_failure i.e., it'd fail if event.original != null? So, with on_failure we can pass the remove processor with no condition. Is this gonna work? If yes, removal would happen in fewer processing.

Yes, that would work as well. We are following this remove practice. See elastic/package-spec#583.

agithomas · 2023-12-01T05:10:16Z

+        show_user: false
+        description: The files to be excluded by the Agent. See [exclude_files](https://www.elastic.co/guide/en/beats/filebeat/current/filebeat-input-log.html#filebeat-input-log-exclude-files) for details.
+        default:
+          - \.gz$


In the original issue,.bz2 is also highlighted. Do you want to consider bz2 also in the default exclusion list?

No, that would be left to the user. As mostly users exclude .gz files

ritalwar · 2023-12-01T05:14:58Z

+      if: 'ctx.event?.original != null' 
+      field: message
+      ignore_missing: true
+      description: 'The `message` field is no longer required if the document has an `event.original` field.'


Just wanted to know, in instances where event.original is already present, such as when it comes from Logstash, we wouldn't alter the event.original field, and it wouldn't be replaced(as per expected state description here). Is there a chance in these situations for the data in the message and event.original fields to be different? If so, I believe we should refrain from removing the message field in those cases and consider adding a check to account for this scenario.

Once it enters the pipeline event.original and message would be same. Hence we would like to avoid having duplicates. Thats why deletion is important immediately after the rename of event original to message field.

There can be a case where message field it used after the removal as seen in syslog datastream. So that also gets handled by the removal of message field before this.

Co-authored-by: subham sarkar <sarkar.subhams2@gmail.com>

devamanv

LGTM

Co-authored-by: muthu-mps <101238137+muthu-mps@users.noreply.github.com>

elasticmachine · 2023-12-01T10:01:28Z

Package system - 1.50.0 containing this change is available at https://epr.elastic.co/search?package=system

elasticmachine · 2023-12-01T10:01:30Z

Package system - 1.50.0 containing this change is available at https://epr.elastic.co/search?package=system

ishleenk17 and others added 9 commits September 22, 2023 12:00

Update null checks anf if checks in the rename

23cf3a5

Update changelog.yml

9bcd85d

Merge branch 'elastic:main' into main

1462704

Merge branch 'elastic:main' into main

3bfa4f3

Merge branch 'elastic:main' into main

7df11eb

Merge branch 'elastic:main' into main

9f4a04a

Merge branch 'elastic:main' into main

58330e6

Merge branch 'elastic:main' into main

5c5a7eb

Syslog datastream changes

7d6b8df

ishleenk17 added the enhancement New feature or request label Nov 30, 2023

ishleenk17 requested review from a team as code owners November 30, 2023 17:49

Update changelog

c2b1455

devamanv changed the title ~~Message Parsing issue and exclusion of files in syslig datastream~~ Message Parsing issue and exclusion of files in syslog datastream Nov 30, 2023

shmsr reviewed Nov 30, 2023

View reviewed changes

Comment thread packages/system/data_stream/syslog/manifest.yml Outdated

shmsr changed the title ~~Message Parsing issue and exclusion of files in syslog datastream~~ system/data_stream/syslog: Message Parsing issue and exclusion of files in syslog datastream Nov 30, 2023

efd6 reviewed Nov 30, 2023

View reviewed changes

Comment thread packages/system/data_stream/syslog/manifest.yml Outdated

agithomas reviewed Dec 1, 2023

View reviewed changes

ritalwar reviewed Dec 1, 2023

View reviewed changes

shmsr reviewed Dec 1, 2023

View reviewed changes

Comment thread packages/system/data_stream/syslog/manifest.yml Outdated

ishleenk17 and others added 2 commits December 1, 2023 11:58

Update packages/system/data_stream/syslog/manifest.yml

298e2ce

Co-authored-by: subham sarkar <sarkar.subhams2@gmail.com>

Update manifest.yml

cdc0471

muthu-mps reviewed Dec 1, 2023

View reviewed changes

Comment thread packages/system/changelog.yml Outdated

shmsr approved these changes Dec 1, 2023

View reviewed changes

devamanv approved these changes Dec 1, 2023

View reviewed changes

Update packages/system/changelog.yml

8419382

Co-authored-by: muthu-mps <101238137+muthu-mps@users.noreply.github.com>

ritalwar approved these changes Dec 1, 2023

View reviewed changes

ishleenk17 merged commit 6ac94ad into elastic:main Dec 1, 2023

andrewkroh added the Integration:system System label Jul 22, 2024

ishleenk17 deleted the system_syslog branch February 7, 2025 03:43

Conversation

ishleenk17 commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed commit message

Checklist

Related issues

Uh oh!

elasticmachine commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

Uh oh!

elasticmachine commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌐 Coverage report

Uh oh!

devamanv commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

shmsr commented Nov 30, 2023

Uh oh!

efd6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ishleenk17 commented Dec 1, 2023

Uh oh!

ishleenk17 commented Dec 1, 2023

Uh oh!

agithomas Dec 1, 2023

Choose a reason for hiding this comment

Uh oh!

ishleenk17 Dec 1, 2023

Choose a reason for hiding this comment

Uh oh!

ritalwar Dec 1, 2023

Choose a reason for hiding this comment

Uh oh!

ishleenk17 Dec 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devamanv left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Dec 1, 2023

Uh oh!

elasticmachine commented Dec 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ishleenk17 commented Nov 30, 2023 •

edited

Loading

elasticmachine commented Nov 30, 2023 •

edited

Loading

elasticmachine commented Nov 30, 2023 •

edited

Loading

devamanv commented Nov 30, 2023 •

edited

Loading