Skip to content
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

Syslog rfc3164 location (timestamp) parameter #247

Merged
merged 3 commits into from
Feb 3, 2021

Conversation

jsirianni
Copy link
Member

@jsirianni jsirianni commented Feb 3, 2021

Description of Changes

Added a location parameter for overriding the default UTC timezone used by the syslog package.

This change is for RFC 3164 syslog only, as the timestamps do not include a timezone. RFC 5424 does not require this parameter, and will ignore it.

This resolves an issue where the agent is receiving syslog messages from a system using a timezone other than UTC for the timestamp.

Initially I thought I would need to modify SyslogParserConfig.ParserConfig.TimeParser, but I found that go-syslog is transforming the raw timestamp into a time.Time value before it reaches the syslog operator.

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Add a changelog entry (for non-trivial bug fixes / features)
  • CI passes

@jsirianni jsirianni marked this pull request as ready for review February 3, 2021 01:20
@djaglowski
Copy link
Member

Log Files Logs / Second CPU Avg (%) CPU Avg Δ (%) Memory Avg (MB) Memory Avg Δ (MB)
1 1000 1.8793439 +0.24137425 123.52196 +0.49892426
1 5000 6.241509 -0.15516043 131.4872 -1.3073883
1 10000 12.20723 +0.034700394 140.42618 -0.45635986
1 50000 57.274982 -0.7234726 170.8529 -10.696396
1 100000 109.24248 -2.3760529 230.77802 -4.732895
10 100 2.327677 -0.017238379 128.2473 +4.695572
10 500 6.586356 -0.18968248 134.58566 -0.75956726
10 1000 13.827895 +0.8276186 144.0039 +0.22885132
10 5000 60.07107 -2.8395462 184.73303 +2.1736298
10 10000 122.56434 -10.776176 219.68845 -17.33136

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

This looks great.

@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #247 (74c69f2) into master (0c54a1d) will increase coverage by 0.01%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
+ Coverage   71.47%   71.48%   +0.01%     
==========================================
  Files         102      102              
  Lines        5598     5604       +6     
==========================================
+ Hits         4001     4006       +5     
  Misses       1168     1168              
- Partials      429      430       +1     
Impacted Files Coverage Δ
operator/builtin/parser/syslog/syslog.go 67.24% <75.00%> (-0.03%) ⬇️
operator/builtin/input/file/file.go 73.97% <0.00%> (-2.05%) ⬇️
operator/builtin/output/forward/forward.go 59.42% <0.00%> (+2.90%) ⬆️
operator/flusher/flusher.go 88.00% <0.00%> (+4.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c54a1d...74c69f2. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants