From a122a0aba6b52720bbb55970cdd3c1d8e35380e4 Mon Sep 17 00:00:00 2001 From: rofinn Date: Tue, 9 Nov 2021 10:14:25 -0800 Subject: [PATCH 1/3] Make TimeZones.jl and Syslogs.jl optional dependencies. AFAICT, this should be breaking because our formatting code still produces the same output. The only differences is that our records are only storing localzone datetimes which are converted to the appropriate timezone when necessary. --- Project.toml | 10 ++++++---- src/Memento.jl | 9 ++++----- src/formatters.jl | 14 +++++--------- src/records.jl | 4 ++-- src/syslog.jl | 2 ++ src/tz.jl | 17 +++++++++++++++++ 6 files changed, 36 insertions(+), 20 deletions(-) create mode 100644 src/tz.jl diff --git a/Project.toml b/Project.toml index d3660fb..11eec06 100644 --- a/Project.toml +++ b/Project.toml @@ -7,22 +7,24 @@ version = "1.3.0" [deps] Dates = "ade2ca70-3891-5945-98fb-dc099432e06a" Distributed = "8ba89e20-285c-5b6f-9357-94700520ee1b" -JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6" +Requires = "ae029012-a4dd-5104-9daa-d747884805df" Serialization = "9e88b42a-f829-5b0c-bbe9-9e923198166b" Sockets = "6462fe0b-24de-5631-8697-dd941f90decc" -Syslogs = "cea106d9-e007-5e6c-ad93-58fe2094e9c4" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" -TimeZones = "f269a46b-ccf7-5d73-abea-4c690281aa53" UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4" [compat] JSON = "0.16.1, 0.17, 0.18, 0.19, 0.20, 0.21" +Requires = "1" Syslogs = "0.0.1, 0.1, 0.2, 0.3" TimeZones = "0.7, 0.8, 0.9, 0.10, 0.11, 1" julia = "1" [extras] +JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6" Suppressor = "fd094767-a336-5f1f-9728-57cf17d0bbfb" +Syslogs = "cea106d9-e007-5e6c-ad93-58fe2094e9c4" +TimeZones = "f269a46b-ccf7-5d73-abea-4c690281aa53" [targets] -test = ["Suppressor"] +test = ["JSON", "Suppressor", "Syslogs", "TimeZones"] diff --git a/src/Memento.jl b/src/Memento.jl index 594a967..7deeeb1 100644 --- a/src/Memento.jl +++ b/src/Memento.jl @@ -5,12 +5,9 @@ using UUIDs using Dates using Sockets using Distributed +using Requires using Serialization -using Syslogs -using JSON -using TimeZones - # We're often extending these methods import Base: error, log @@ -45,7 +42,6 @@ include("filters.jl") include("formatters.jl") include("handlers.jl") include("loggers.jl") -include("syslog.jl") include("stdlib.jl") include("config.jl") include("exceptions.jl") @@ -64,6 +60,9 @@ const LOGGER = getlogger(@__MODULE__) function __init__() Memento.config!(DEFAULT_LOG_LEVEL) Memento.register(LOGGER) + + @require TimeZones="f269a46b-ccf7-5d73-abea-4c690281aa53" include("tz.jl") + @require Syslogs="cea106d9-e007-5e6c-ad93-58fe2094e9c4" include("syslog.jl") end _precompile_() diff --git a/src/formatters.jl b/src/formatters.jl index 0d25970..5ee95ec 100644 --- a/src/formatters.jl +++ b/src/formatters.jl @@ -79,15 +79,7 @@ function format(fmt::DefaultFormatter, rec::Record) value = string(" stack:[", join(str_frames, ", "), "]") elseif content === :date - value = if tmp_val isa ZonedDateTime - # `localzone` is expensive, so we don't call it until it is required. - tzout = fmt.output_tz === nothing ? localzone() : fmt.output_tz - Dates.format(astimezone(tmp_val, tzout), fmt.date_fmt_string) - elseif tmp_val isa DateTime - Dates.format(tmp_val, fmt.date_fmt_string) - else - tmp_val - end + value = _format_datetime(tmp_val, fmt.date_fmt_string, fmt.output_tz) else value = tmp_val end @@ -172,3 +164,7 @@ function format(fmt::DictFormatter, rec::Record) return fmt.serializer(dict) end + +# This is overloaded to return a ZonedDateTime only when an output_tz is specified +_format_datetime(dt, args...) = dt +_format_datetime(dt::DateTime, fmt::AbstractString, tz::Nothing) = Dates.format(dt, fmt) diff --git a/src/records.jl b/src/records.jl index 9101fda..daca55f 100644 --- a/src/records.jl +++ b/src/records.jl @@ -120,7 +120,7 @@ NOTE: if you'd like more logging attributes you can: 2. make a custom `Record` type. # Fields -* `date::Attribute{ZonedDateTime}`: timestamp of log event +* `date::Attribute{DateTime}`: timestamp of log event * `level::Attribute{AbstractString}`: log level * `levelnum::Attribute{Int}`: integer value for log level * `msg::Attribute{AbstractString}`: the log message itself @@ -157,7 +157,7 @@ function DefaultRecord(name::AbstractString, level::AbstractString, levelnum::In trace = Attribute{StackTrace}(get_trace) DefaultRecord( - Attribute{ZonedDateTime}(() -> Dates.now(tz"UTC")), + Attribute{DateTime}(() -> Dates.now()), Attribute(level), Attribute(levelnum), Attribute{AbstractString}(msg), diff --git a/src/syslog.jl b/src/syslog.jl index 3b27a03..7367a58 100644 --- a/src/syslog.jl +++ b/src/syslog.jl @@ -1,3 +1,5 @@ +using Syslogs + # This is necessary or Memento will attempt to use `println` with one argument (unsupported by Syslog) # This should be handled by optional dependency stuff in the future """ diff --git a/src/tz.jl b/src/tz.jl new file mode 100644 index 0000000..2b0f89b --- /dev/null +++ b/src/tz.jl @@ -0,0 +1,17 @@ +using TimeZones + +# We're just adding one timezone specific method for constructing and formatting a ZonedDateTime +function _format_datetime(dt::DateTime, fmt::AbstractString, tz::Dates.TimeZone) + return Dates.format(ZonedDateTime(dt, tz), fmt) +end + +# Since we already default to local zone in the base case explicitly convert a +# ZonedDateTime to be consistent. +function _format_datetime(zdt::ZonedDateTime, fmt::AbstractString, tz::Nothing) + return Dates.format(astimezone(zdt, localzone()), fmt) +end + +# Convert ZonedDateTimes to a specified output tz +function _format_datetime(zdt::ZonedDateTime, fmt::AbstractString, tz::Dates.TimeZone) + return Dates.format(astimezone(zdt, tz), fmt) +end From d456d8fdc1de1bf5b8fcbfbc587508368b98d2be Mon Sep 17 00:00:00 2001 From: rofinn Date: Tue, 9 Nov 2021 11:24:23 -0800 Subject: [PATCH 2/3] Use relative import to avoid package loading warning. --- src/syslog.jl | 2 +- src/tz.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/syslog.jl b/src/syslog.jl index 7367a58..881fe73 100644 --- a/src/syslog.jl +++ b/src/syslog.jl @@ -1,4 +1,4 @@ -using Syslogs +using .Syslogs # This is necessary or Memento will attempt to use `println` with one argument (unsupported by Syslog) # This should be handled by optional dependency stuff in the future diff --git a/src/tz.jl b/src/tz.jl index 2b0f89b..44ad562 100644 --- a/src/tz.jl +++ b/src/tz.jl @@ -1,4 +1,4 @@ -using TimeZones +using .TimeZones # We're just adding one timezone specific method for constructing and formatting a ZonedDateTime function _format_datetime(dt::DateTime, fmt::AbstractString, tz::Dates.TimeZone) From d4b2fbca58ebbc5b29c80b6383450a6b99572d2c Mon Sep 17 00:00:00 2001 From: rofinn Date: Wed, 10 Nov 2021 13:21:15 -0800 Subject: [PATCH 3/3] Add test for dispatch with DateTime and output_tz. --- src/tz.jl | 2 +- test/formatters.jl | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tz.jl b/src/tz.jl index 44ad562..ade0d36 100644 --- a/src/tz.jl +++ b/src/tz.jl @@ -2,7 +2,7 @@ using .TimeZones # We're just adding one timezone specific method for constructing and formatting a ZonedDateTime function _format_datetime(dt::DateTime, fmt::AbstractString, tz::Dates.TimeZone) - return Dates.format(ZonedDateTime(dt, tz), fmt) + return Dates.format(astimezone(ZonedDateTime(dt, localzone()), tz), fmt) end # Since we already default to local zone in the base case explicitly convert a diff --git a/test/formatters.jl b/test/formatters.jl index be05e2b..dbe18c6 100644 --- a/test/formatters.jl +++ b/test/formatters.jl @@ -48,6 +48,7 @@ date = DateTime(2012, 1, 1, 3, 1, 1) notz_rec = SimpleRecord("info", "blah", date) @test Memento.format(fmt, notz_rec) == local_datestr + @test Memento.format(utc_fmt, notz_rec) == utc_datestr ts = 1531949014 ts_rec = SimpleRecord("info", "blah", ts)