Add allow skip pri otion to syslog. #30869
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
cmd/configschema
github-actions
bot
requested review from
armstrmi,
atoulme,
dehaansa,
dineshg13,
djaglowski,
dmitryax and
jpkrohling
|
Thanks! Just a couple of nits. |
Co-authored-by: Andrzej Stencel <astencel@sumologic.com>
Co-authored-by: Andrzej Stencel <astencel@sumologic.com>
thanks @astencel-sumo |
djaglowski
approved these changes
Feb 8, 2024
|
sorry to bother you, but the unit test seems to be unstable. during the last couple of days, I pushed many changes, sometimes the build failed, and sometimes it passed.
|
|
A PR was just merged to roll back a the change that caused the test failures. I've just updated this PR so it should pass now. |
thanks! |
anthoai97
pushed a commit
to anthoai97/opentelemetry-collector-contrib
that referenced
this pull request
Feb 12, 2024
**Description:** Currently syslog parser (and receiver) are enforcing a pri header at the beginning of each syslog message. This behavior is incorrect since syslog RFC doesn't require a pri header. to fix this I added a new 'allow_skip_pri_header' (default false) to syslog config to allow users to choose if they want to allow the parser to skip the pri existence validation. the main issue was that the syslog parser is using influxdata/go-syslog library which unfortunately is not maintained anymore (according to the repo PR history). we had a couple of options to fix it and after a long discussion [here](open-telemetry#30397) we have decided to move to a new forked repo. so I forked the repo, made the necessary changes and updated the syslog parser to start using the new repo. after that, I ran: make goproto, make crosslink, make -j8 gotidy. and as a result many files (mostly go.mod and go.sum) files changed. **Link to tracking Issue:** open-telemetry#30397 **Testing:** Manual testing for both RFC 3164 and 5424 with and without pri header. --------- Co-authored-by: Andrzej Stencel <astencel@sumologic.com> Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
LokeshOpsramp
pushed a commit
to opsramp/opentelemetry-collector-contrib
that referenced
this pull request
May 5, 2024
**Description:** Currently syslog parser (and receiver) are enforcing a pri header at the beginning of each syslog message. This behavior is incorrect since syslog RFC doesn't require a pri header. to fix this I added a new 'allow_skip_pri_header' (default false) to syslog config to allow users to choose if they want to allow the parser to skip the pri existence validation. the main issue was that the syslog parser is using influxdata/go-syslog library which unfortunately is not maintained anymore (according to the repo PR history). we had a couple of options to fix it and after a long discussion [here](open-telemetry#30397) we have decided to move to a new forked repo. so I forked the repo, made the necessary changes and updated the syslog parser to start using the new repo. after that, I ran: make goproto, make crosslink, make -j8 gotidy. and as a result many files (mostly go.mod and go.sum) files changed. **Link to tracking Issue:** open-telemetry#30397 **Testing:** Manual testing for both RFC 3164 and 5424 with and without pri header. --------- Co-authored-by: Andrzej Stencel <astencel@sumologic.com> Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: Currently syslog parser (and receiver) are enforcing a pri header at the beginning of each syslog message. This behavior is incorrect since syslog RFC doesn't require a pri header.
to fix this I added a new 'allow_skip_pri_header' (default false) to syslog config to allow users to choose if they want to allow the parser to skip the pri existence validation.
the main issue was that the syslog parser is using influxdata/go-syslog library which unfortunately is not maintained anymore (according to the repo PR history).
we had a couple of options to fix it and after a long discussion here we have decided to move to a new forked repo.
so I forked the repo, made the necessary changes and updated the syslog parser to start using the new repo.
after that, I ran: make goproto, make crosslink, make -j8 gotidy.
and as a result many files (mostly go.mod and go.sum) files changed.
Link to tracking Issue: #30397
Testing: Manual testing for both RFC 3164 and 5424 with and without pri header.