Skip to content

FileTarget - Replace Environment.TickCount with LogEventInfo.TimeStamp#6079

Merged
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:FileTargetNoTickCount
Jan 22, 2026
Merged

FileTarget - Replace Environment.TickCount with LogEventInfo.TimeStamp#6079
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:FileTargetNoTickCount

Conversation

@snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Jan 22, 2026 •
edited
Loading

Skip operating system call for coarse timestamp, when already have LogEventInfo.TimeStamp (NET11 will make Environment.TickCount slower on Windows in exchange for higher time-precision)

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026 •
edited
Loading

Walkthrough

Adds a DateTime timestamp parameter to IFileAppender.Write and all implementations; FileTarget now supplies timestamps when writing. ExclusiveFileLockingAppender replaces tick-based deletion checks with timestamp-based logic and a new HasFileBeenDeletedTime helper.

Changes

Cohort / File(s) Summary
Interface Definition
src/NLog/Targets/FileAppenders/IFileAppender.cs		 Write signature changed to Write(DateTime timestamp, byte[] buffer, int offset, int count); XML doc updated.
Simple Implementations
src/NLog/Targets/FileAppenders/DiscardAllFileAppender.cs, src/NLog/Targets/FileAppenders/MinimalFileLockingAppender.cs		 Write signature updated to include DateTime timestamp as first parameter; bodies unchanged (timestamp currently unused).
Complex Implementation
src/NLog/Targets/FileAppenders/ExclusiveFileLockingAppender.cs		 Write signature updated to include DateTime timestamp. _lastFileDeletedCheck changed from int to DateTime and initialized from OpenStreamTime. Added HasFileBeenDeletedTime(DateTime timestamp) which computes time deltas and conditionally triggers MonitorFileHasBeenDeleted. FileLastModified updated within the new timing logic.
Integration Point
src/NLog/Targets/FileTarget.cs		 Calls to file appender Write updated to pass timestamps (e.g., Write(firstLogEvent.TimeStamp, ms.GetBuffer(), 0, (int)ms.Length) and Write(openFile.FileAppender.FileLastModified, footerBytes, 0, footerBytes.Length)).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant FileTarget
participant IFileAppender
participant ExclusiveAppender
participant FS as FileSystem
FileTarget->>IFileAppender: Write(timestamp, buffer, offset, count)
IFileAppender->>ExclusiveAppender: Write(timestamp, buffer, offset, count)
ExclusiveAppender->>ExclusiveAppender: HasFileBeenDeletedTime(timestamp)
alt file deletion suspected
ExclusiveAppender->>ExclusiveAppender: MonitorFileHasBeenDeleted()
ExclusiveAppender->>FS: check file metadata/size
FS-->>ExclusiveAppender: metadata/size
end
ExclusiveAppender->>FS: write bytes to file
FS-->>ExclusiveAppender: write result
ExclusiveAppender-->>IFileAppender: return
IFileAppender-->>FileTarget: return

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nibble timestamps into every write,
Hopping through bytes by soft moonlight,
Ticks replaced with moments clear and bright,
A gentle check keeps files polite,
Cheers from a rabbit—code done right! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title clearly summarizes the main change: replacing Environment.TickCount with LogEventInfo.TimeStamp in FileTarget to improve performance by avoiding redundant OS timestamp calls.
Description check ✅ Passed The pull request description directly relates to the changeset, which replaces Environment.TickCount with LogEventInfo.TimeStamp throughout file appenders.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@snakefoot snakefoot force-pushed the FileTargetNoTickCount branch from 2dc86ef to 8e6e689 Compare January 22, 2026 19:10
coderabbitai[bot]
coderabbitai bot reviewed Jan 22, 2026
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents 
In `@src/NLog/Targets/FileAppenders/ExclusiveFileLockingAppender.cs`:
- Around line 128-153: HasFileBeenDeletedTime currently only advances when the
provided timestamp moves ±1s from _lastFileDeletedCheck, which stalls checks if
callers reuse LogEventInfo timestamps; change HasFileBeenDeletedTime (used by
Write) to also compare the wall‑clock (e.g. NLog.Time.TimeSource.Current.Time)
against _lastFileDeletedCheck and trigger the delete/modified logic when the
wall‑clock has advanced more than the threshold even if the input timestamp
hasn’t; update _lastFileDeletedCheck and set FileLastModified from the
wall‑clock in that fallback path so periodic checks still run when event
timestamps are reused or low resolution.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 22, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@snakefoot snakefoot merged commit aa6ea68 into NLog:dev Jan 22, 2026
5 of 6 checks passed
@snakefoot snakefoot mentioned this pull request Jan 24, 2026
Merged
@snakefoot snakefoot added this to the 6.1.0 milestone Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

file-target performance size/M

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

1 participant

@snakefoot