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

Add special syslog timestamp parser that uses current year #4190

Merged
merged 1 commit into from
May 23, 2018

Conversation

danielnelson
Copy link
Contributor

@danielnelson danielnelson commented May 23, 2018

Previously it was impossible to parse syslog timestamps without the date being reported as year 0, due to the year not being specified.

This pull request adds the special timestamp format: ts-syslog that will set the year after parsing to the current year. %{SYSLOGTIMESTAMP:timestamp:ts-syslog}

closes #3379

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Previously it was impossible to parse syslog timestamps without the date
being reported as year 0, due to the year not being specified
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label May 23, 2018
@danielnelson danielnelson added this to the 1.7.0 milestone May 23, 2018
@danielnelson danielnelson merged commit 14d97e5 into master May 23, 2018
@danielnelson danielnelson deleted the logparser-syslog branch May 23, 2018 23:37
@@ -104,6 +104,7 @@ You must capture at least one field per line.
- ts-httpd ("02/Jan/2006:15:04:05 -0700")
- ts-epoch (seconds since unix epoch, may contain decimal)
- ts-epochnano (nanoseconds since unix epoch)
- ts-syslog ("Jan 02 15:04:05", parsed time is set to the current year)
Copy link
Contributor

Choose a reason for hiding this comment

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

@danielnelson this is the BSD syslog timestamp format (RFC3164) not the syslog RFC5424 timestamp (basically a RFC3339 micro timestamp).
So probably you want to call it ts-bsd-syslog, and maybe also add another format for ts-syslog.
Just my 2 cents.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that both of these RFCs describe the wire format for syslog messages, and not how the time might be written to logfiles. Is there any documention that describes how the timestamp in, for instance, /var/log/syslog are written?

Another concern I had, and maybe you can help with, is if these dates are commonly localized?

Copy link
Contributor

@leodido leodido May 25, 2018

Choose a reason for hiding this comment

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

The timestamp depends on the format (so RFC) chosen.
For example you can configure RSYSLOG to use RSYSLOG_SyslogProtocol23Format format, which means RFC5424. With this setting /var/log/* will contains RFC3339MICRO timestamps (as per RFC5424). Without this setting it will default to the old (RFC3164) BSD syslog (and timestamp) format.

Copy link
Contributor

@leodido leodido May 25, 2018

Choose a reason for hiding this comment

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

Regarding localization. RFC5425 with RFC3339MICRO timestamps is not affected.
Rather, timestamps of syslog messages following the old RFC3164 are not localized (must be always in english) (ref).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll leave this as ts-syslog since it pairs with the built in grok pattern SYSLOGTIMESTAMP and there isn't any reason to add a special pattern for rfc5424 style timestamps since we already have ts-rfc3339 and ts-rfc3339nano. We only need a special type here to inject the year.

@@ -285,6 +292,16 @@ func (p *Parser) ParseLine(line string) (telegraf.Metric, error) {
} else {
timestamp = time.Unix(0, iv)
}
case SYSLOG_TIMESTAMP:
ts, err := time.ParseInLocation("Jan 02 15:04:05", v, p.loc)
Copy link

Choose a reason for hiding this comment

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

Hi @danielnelson
Should we use "Jan 2 15:04:05" instead of "Jan 02 15:04:05" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think maybe it should be Jan _2 15:04:05, but I can't seem to create a testcase that fails. Do you have a example that is failing?

Copy link

Choose a reason for hiding this comment

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

If we try to parse "Sep 2 09:01:55 value=42" in test function TestSyslogTimestampParser(),
the test will show the following:
=== RUN TestSyslogTimestampParser 2018/06/22 09:36:41 E! Error parsing Sep 2 09:01:55 to time layout [SYSLOG_TIMESTAMP]: parsing time "Sep 2 09:01:55" as "Jan 02 15:04:05": cannot parse "2 09:01:55" as "02"

Does this the example count?
Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, the test was "passing" even though the parsing failed. Here is the fix: #4334

Copy link

Choose a reason for hiding this comment

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

That fix looks nice. Thank you.

maxunt pushed a commit that referenced this pull request Jun 26, 2018
Previously it was impossible to parse syslog timestamps without the date
being reported as year 0, due to the year not being specified
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
…a#4190)

Previously it was impossible to parse syslog timestamps without the date
being reported as year 0, due to the year not being specified
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logparser : Epoch convertion impossible with SYSLOGTIMESTAMP Grok pattern
3 participants