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.

Sign up for GitHub

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

Jump to bottom

add RfcUtcTime compilation option #152

Merged
merged 3 commits into from
Sep 19, 2024
Merged

add RfcUtcTime compilation option #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
580f64a
log_output: use of getFastDateTimeString with RfcUtcTime format is set
Ivansete-status Sep 4, 2024
9efb9d6
README.md: move RfcUtcTime to a more appropriate place
Ivansete-status Sep 19, 2024
f4980cf
log_output: avoid innecessary default value in getFastDateTimeString …
Ivansete-status Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,13 @@ Possible values are:

https://tools.ietf.org/html/rfc3339

- `RfcUtcTime`
Ivansete-status marked this conversation as resolved.
Show resolved Hide resolved

Chronicles will use the UTC but in human-readable format specified in
RFC 3339: Date and Time on the Internet: Timestamps

https://tools.ietf.org/html/rfc3339

- `UnixTime`

Chronicles will write a single float value for the number
Expand Down
22 changes: 14 additions & 8 deletions chronicles/log_output.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,29 +454,35 @@ proc getSecondsPart(timestamp: Time): string =
res[5] = chr(ord('0') + (tmp mod 10))
res

proc getFastDateTimeString(): string =
proc getFastDateTimeString(useUtc: bool): string =
## if useUtc is true, utc time is used and Z for the timezone part.
## if useUtc is false, the local time will be used and the time zone will be obtained each time.
let
timestamp = getFastTime()
minutes = timestamp.toUnix() div 60

if minutes != cachedMinutes:
cachedMinutes = minutes
let datetime = timestamp.local()
let datetime = if useUtc: timestamp.utc() else: timestamp.local()
block:
# Cache string representation of first part (without seconds)
let tmp = datetime.format("yyyy-MM-dd HH:mm:")
cachedTimeArray = toArray(17, tmp.toOpenArrayByte(0, 16))
block:
# Cache string representation of zone part
let tmp = datetime.format("zzz")
cachedZoneArray = toArray(6, tmp.toOpenArrayByte(0, 5))
if not useUtc:
Copy link
Member

Choose a reason for hiding this comment

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

there's an if for every formatting statement here - maybe accept that these are separate functions..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks so much indeed for the comments!
For this particular one, I think it is better to keep it in the same proc to avoid possible leftovers if we change that logic in the future.

Copy link
Member

@arnetheduck arnetheduck Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

even so, most of it can be avoided..

let timezone =
  if useUtc: "Z"
  else:
     ...
     string.fromBytes(cachedZoneArray)

# Cache string representation of zone part
let tmp = datetime.format("zzz")
cachedZoneArray = toArray(6, tmp.toOpenArrayByte(0, 5))

string.fromBytes(cachedTimeArray) & timestamp.getSecondsPart() &
string.fromBytes(cachedZoneArray)
let timeZone = if useUtc: "Z" else: string.fromBytes(cachedZoneArray)

string.fromBytes(cachedTimeArray) & timestamp.getSecondsPart() & timeZone

template timestamp(record): string =
when record.timestamps == RfcTime:
getFastDateTimeString()
getFastDateTimeString(useUtc = false)
elif record.timestamps == RfcUtcTime:
getFastDateTimeString(useUtc = true)
else:
epochTimestamp()

Expand Down
4 changes: 3 additions & 1 deletion chronicles/options.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ type
TimestampsScheme* = enum
NoTimestamps,
UnixTime,
RfcTime
RfcTime,
RfcUtcTime

ColorScheme* = enum
NoColors,
Expand Down Expand Up @@ -257,6 +258,7 @@ proc sinkSpecsFromNode*(streamNode: NimNode): seq[SinkSpec] =
of "notimestamps": setTimestamps(NoTimestamps)
of "unixtime": setTimestamps(UnixTime)
of "rfctime": setTimestamps(RfcTime)
of "rfcutctime": setTimestamps(RfcUtcTime)
else: discard

let dst = logDestinationFromNode(dstSpec)
Expand Down