Conversation
abh
commented
Jul 28, 2025
abh
commented
- English comments and logs
- Add more logs about the current responses
- Make processing delay configurable
- Add total offset to the logs
- Add persistence so drift doesn't reset on restart
- Log current drift rate (ppm) and cumulative offset (ms) - Log actual jitter value applied to each response - Log reference time offset for complete timing picture - Enhanced createFakeNTPResponse to return timing values
- Add ProcessingDelayMs field to Config struct - Replace hardcoded 10ms delay with configurable value - Update debug logging to show processing delay setting - Allows simulation of different server processing times
- Move NTP version inline with client IP for compact display - Calculate total time offset from real time (drift + jitter) - Log all offset components with total offset shown first - Add comment clarifying RefTime doesn't affect client sync - Provide complete visibility into timing manipulations
- Add RuntimeState struct for drift and random state - Save/load state to JSON file on startup/shutdown - Maintain consistent responses across restarts - Add --reset-state flag to start fresh - Use deterministic random values with saved seed
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the fake NTP server with improved configuration, logging, persistence, and internationalization. The changes focus on making the server more realistic and maintainable by adding English translations, configurable processing delays, state persistence across restarts, and enhanced debugging output.
- Translates all Dutch comments and log messages to English
- Adds state persistence functionality to maintain drift across server restarts
- Implements configurable processing delay and enhanced logging with offset information
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| fake-ntpd.go | Main implementation with English translations, state persistence, configurable processing delay, and enhanced logging |
| config.json | Updated configuration file with new state persistence options |
| // Create fresh state if not loaded | ||
| if driftSim == nil { | ||
| seed := time.Now().UnixNano() | ||
| rand.Seed(seed) |
There was a problem hiding this comment.
Using the global rand.Seed() is deprecated and not thread-safe. Since you're already creating a local rand.Rand instance on line 340, you should remove this line and only use the local instance.
| rand.Seed(seed) |
| // (RefTime doesn't affect client sync, only TxTime matters) | ||
| totalOffset := driftOffset + jitter | ||
|
|
||
| refTime := now.Add(-time.Duration(cfg.MaxRefTimeOffset) * time.Second) |
There was a problem hiding this comment.
The refTime calculation uses a hardcoded negative offset while refTimeOffset variable is calculated but not used. Consider using the refTimeOffset variable for consistency: refTime := now.Add(-refTimeOffset)
| refTime := now.Add(-time.Duration(cfg.MaxRefTimeOffset) * time.Second) | |
| refTime := now.Add(-refTimeOffset) |
| } | ||
|
|
||
| if d.model == "random_walk" && time.Since(d.lastUpdate) >= d.updateEvery { | ||
| // Use the global random source seeded by the main function |
There was a problem hiding this comment.
This comment is incorrect since the code now uses a local rand.Rand instance passed as a parameter, not a global random source. The comment should be updated to reflect the current implementation.
| // Use the global random source seeded by the main function | |
| // Use the local rand.Rand instance passed as a parameter |
