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

date-parser: extend %z to support time zones other then local and gmt #3453

Merged
merged 4 commits into from Nov 11, 2020

Conversation

Kokan
Copy link
Collaborator

@Kokan Kokan commented Oct 12, 2020

Our date parser is based on the NetBSD strptime implementation, that differs from the linux implementation.
The version of strptime used from NetBSD supported local and GMT timezone with %z and the %Z supported the other timezones. This was changed in linux and also in later version of NetBSD.

In our documentation the newer NetBSD behaviour was written, this patch updates to code accordingly.

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@bazsi
Copy link
Collaborator

bazsi commented Oct 12, 2020

I think this drops support for recognizing the name of the timezone that is currently set on the current host running syslog-ng.

e.g. the tzname global variable that is set by libc.

Try setting CET as the current timezone and send a message using "CET" as the timezone name.

I am not sure this is that useful, but Ciscos tend to send timezone names instead of offsets and at the very least those would be recognized as long as the router is in the same timezone. In contrast to this we should attempt to recognize any timezone names, that we could extract from tzinfo files, as we do with allowed timezone offsets (e.g. _is_gmtoff_valid() function in unixtime.c)

@Kokan Kokan changed the title date-parser: extend %z to support time zones other then local and gmt WIP: date-parser: extend %z to support time zones other then local and gmt Oct 13, 2020
Signed-off-by: Kokan <kokaipeter@gmail.com>
Porting NetBSD libc:
NetBSD/src@1ab7665

Signed-off-by: Kokan <kokaipeter@gmail.com>
Signed-off-by: Kokan <kokaipeter@gmail.com>
Signed-off-by: Kokan <kokaipeter@gmail.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

@Kokan Kokan changed the title WIP: date-parser: extend %z to support time zones other then local and gmt date-parser: extend %z to support time zones other then local and gmt Nov 2, 2020
@MrAnno
Copy link
Collaborator

MrAnno commented Nov 5, 2020

Nice!

@bazsi
Copy link
Collaborator

bazsi commented Nov 5, 2020

@rfaircloth-splunk Once we discussed syslog-ng's date-parser() "%Z" format parsing timezone names. This branch improves that with the US based timezone names, still not global, but stuff like EST/EDT would be recognized with this branch integrated. Do you also see non-US timezones in the wild? I guess you do, I just wanted to gauge if it makes sense to support those as well.

Copy link
Collaborator

@bazsi bazsi left a comment

Choose a reason for hiding this comment

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

Could still be improved with all the global timezone names, but still a useful step.

@Kokan
Copy link
Collaborator Author

Kokan commented Nov 11, 2020

There is already two approve and green CI, so I am merging this.

@Kokan Kokan added the user-visible-feature User visible feature label Nov 11, 2020
@Kokan Kokan merged commit d5942b2 into syslog-ng:master Nov 11, 2020
@Kokan Kokan deleted the date-parser-with-z-format branch November 11, 2020 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user-visible-feature User visible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants