Skip to content

Support RFC3339/ISO8601-formatted timestamps when parsing syslog messages in logengine#3723

Merged
lunkwill42 merged 8 commits into5.16.xfrom
bugfix/logengine-timestamp-rfc3339-parse
Jan 8, 2026
Merged

Support RFC3339/ISO8601-formatted timestamps when parsing syslog messages in logengine#3723
lunkwill42 merged 8 commits into5.16.xfrom
bugfix/logengine-timestamp-rfc3339-parse

Conversation

@lunkwill42
Copy link
Copy Markdown
Member

@lunkwill42 lunkwill42 commented Jan 7, 2026

Scope and purpose

Fixes #3722.

The timestamp parsing was already needlessly complex and rife with regexp-crap (I believe this was once ported directly from a Perl script without much change). I felt compelled to refactor it somewhat along the way, in order to not introduce even more complex timestamp pattern matching and match group manipulations (still need patterns to match what looks like timestamps, though).

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation
  • Linted/formatted the code with ruff, easiest by using pre-commit
  • Wrote the commit message so that the first line continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/
  • Based this pull request on the correct upstream branch: For a patch/bugfix affecting the latest stable version, it should be based on that version's branch (<major>.<minor>.x). For a new feature or other additions, it should be based on master.
  • If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
  • If it's not obvious from a linked issue, described how to interact with NAV in order for a reviewer to observe the effects of this change first-hand (commands, URLs, UI interactions)
  • If this results in changes in the UI: Added screenshots of the before and after
  • If this adds a new Python source code file: Added the boilerplate header to that file

Logengine is not able to parse log messages that use RFC 3339
(ISO 8601) formatted timestamps. Adding this single log line to the test
data exposes the problem.
Instead of parsing timestamps as regexp match groups, I intend to
identify and feed the full timestamp string to this function, which
would be more flexible in how timestamps are actually parsed in the
logs.
This removes complicated group matching of timestamp strings, adding
RFC3339 timestamp support and delegates matching strings to the new
parse_timestamp function.
The new timestamp parser supports returning timestamps with
microseconds, since these are supported in the RFC 3339 formats.  The
tests need to take that into account.
Legacy NAV code is timezone-naive and stores timestamps in the local
system time. However, that doesn't mean we should ignore timezone
information in incoming data timestamps - we should at least convert
those timestamps to the local time so we can expect them to be
consistent with other NAV timestamps.
@lunkwill42 lunkwill42 force-pushed the bugfix/logengine-timestamp-rfc3339-parse branch from 4263719 to b42a285 Compare January 7, 2026 13:27
@lunkwill42 lunkwill42 changed the base branch from master to 5.16.x January 7, 2026 13:27
@lunkwill42 lunkwill42 self-assigned this Jan 7, 2026
@lunkwill42 lunkwill42 requested review from a team, aleksfl and hmpf and removed request for aleksfl January 7, 2026 13:32
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 7, 2026

Test results

    27 files      27 suites   46m 23s ⏱️
 2 776 tests  2 776 ✅ 0 💤 0 ❌
20 598 runs  20 598 ✅ 0 💤 0 ❌

Results for commit fd678b5.

♻️ This comment has been updated with latest results.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.03%. Comparing base (9353e01) to head (d5a0c52).
⚠️ Report is 30 commits behind head on 5.16.x.

Additional details and impacted files
@@            Coverage Diff             @@
##           5.16.x    #3723      +/-   ##
==========================================
- Coverage   63.03%   63.03%   -0.01%     
==========================================
  Files         614      614              
  Lines       45420    45438      +18     
  Branches       43       43              
==========================================
+ Hits        28631    28641      +10     
- Misses      16779    16787       +8     
  Partials       10       10              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Hopefully we will be dropping Python 3.9 support soon, but for now we
keep this.
@lunkwill42 lunkwill42 marked this pull request as ready for review January 7, 2026 14:12
@hmpf hmpf self-requested a review January 8, 2026 08:42
hmpf
hmpf previously approved these changes Jan 8, 2026
@hmpf hmpf requested a review from johannaengland January 8, 2026 08:43
johannaengland
johannaengland previously approved these changes Jan 8, 2026
@lunkwill42 lunkwill42 dismissed stale reviews from johannaengland and hmpf via d5a0c52 January 8, 2026 14:04
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jan 8, 2026

@lunkwill42
Copy link
Copy Markdown
Member Author

Stupid GitHub, I did no such thing :D
image

@lunkwill42 lunkwill42 merged commit 6222ae5 into 5.16.x Jan 8, 2026
15 of 16 checks passed
@lunkwill42 lunkwill42 deleted the bugfix/logengine-timestamp-rfc3339-parse branch January 8, 2026 14:05
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.

[BUG] Logengine cannot parse log messages from rsyslog on Debian Bookworm+

3 participants