Skip to content

Commit

Permalink
Reinstate statically computed log record ids
Browse files Browse the repository at this point in the history
Log ids are meant to identify the location of the message in the source
code and must be computed at compile time.

Fixes #28786, #28400; replaces #29355.
  • Loading branch information
c42f committed Nov 1, 2018
1 parent d3a7e83 commit 4cbf55e
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 19 deletions.
24 changes: 11 additions & 13 deletions base/logging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -222,17 +222,17 @@ macro error(exs...) logmsg_code((@_sourceinfo)..., :Error, exs...) end
@eval @doc $_logmsg_docs :(@error)

_log_record_ids = Set{Symbol}()
# Generate a unique, stable, short, human readable identifier for a logging
# statement. The idea here is to have a key against which log records can be
# filtered and otherwise manipulated. The key should uniquely identify the
# source location in the originating module, but should be stable across
# versions of the originating module, provided the log generating statement
# itself doesn't change.
function log_record_id(_module, level, message_ex)
# Generate a unique, stable, short, somewhat human readable identifier for a
# logging *statement*. The idea here is to have a key against which log events
# can be filtered and otherwise manipulated. The key should uniquely identify
# the source location in the originating module, but ideally should be stable
# across versions of the originating module, provided the log generating
# statement itself doesn't change.
function log_record_id(_module, level, message, log_kws)
modname = _module === nothing ? "" : join(fullname(_module), "_")
# Use (1<<31) to fit well within an (arbitriraly chosen) eight hex digits,
# as we increment h to resolve any collisions.
h = hash(string(modname, level, message_ex)) % (1<<31)
# Use an arbitriraly chosen eight hex digits here. TODO: Figure out how to
# make the id exactly the same on 32 and 64 bit systems.
h = UInt32(hash(string(modname, level, message, log_kws)) & 0xFFFFFFFF)
while true
id = Symbol(modname, '_', string(h, base = 16, pad = 8))
# _log_record_ids is a registry of log record ids for use during
Expand All @@ -249,7 +249,7 @@ end

# Generate code for logging macros
function logmsg_code(_module, file, line, level, message, exs...)
id = nothing
id = Expr(:quote, log_record_id(_module, level, message, exs))
group = nothing
kwargs = Any[]
for ex in exs
Expand Down Expand Up @@ -289,8 +289,6 @@ function logmsg_code(_module, file, line, level, message, exs...)
end
end

# Note that it may be necessary to set `id` and `group` manually during bootstrap
id = something(id, :(log_record_id(_module, level, $exs)))
if group == nothing
group = if isdefined(Base, :basename) && isa(file, String)
# precompute if we can
Expand Down
2 changes: 1 addition & 1 deletion stdlib/Logging/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import Logging: min_enabled_level, shouldlog, handle_message
handle_message(logger, Logging.Info, "msg", Base, :group, :asdf, "somefile", 1, maxlog=2)
@test shouldlog(logger, Logging.Info, Base, :group, :asdf) === false

# Check that maxlog works without an explicit ID
# Check that maxlog works without an explicit ID (#28786)
buf = IOBuffer()
io = IOContext(buf, :displaysize=>(30,80), :color=>false)
logger = ConsoleLogger(io)
Expand Down
7 changes: 2 additions & 5 deletions test/logging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ end
@test record.file == Base.source_path()
@test record.line == kwargs[:real_line]
@test record.id isa Symbol
@test occursin(r"^.*[[:xdigit:]]{8}$", String(record.id))
@test occursin(r"^.*logging_[[:xdigit:]]{8}$", String(record.id))

# User-defined metadata
@test kwargs[:bar_val] === bar_val
Expand Down Expand Up @@ -338,11 +338,8 @@ end
@test_logs (:warn, "a", Base) (@warn "a" _module=Base)
end

# Issue #28786
@testset "ID generation" begin
id1 = Base.CoreLogging.log_record_id(Main, "test.jl", 42, Info, ())
id2 = Base.CoreLogging.log_record_id(Main, "test.jl", 42, Info, ())
@test id1 == id2

logs,_ = collect_test_logs() do
for i in 1:2
@info "test"
Expand Down

0 comments on commit 4cbf55e

Please sign in to comment.