From 901600260c553b9407aed893ced5201b3a8f104d Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Sun, 19 Feb 2017 00:21:30 -0600 Subject: [PATCH] Add tags properly --- lib/timber/contexts.rb | 1 - lib/timber/contexts/tags.rb | 22 ---------- lib/timber/log_entry.rb | 13 +++--- lib/timber/logger.rb | 14 +++++- .../probes/active_support_tagged_logging.rb | 43 ------------------- spec/timber/log_devices/http_spec.rb | 8 ++-- spec/timber/log_entry_spec.rb | 4 +- spec/timber/logger_spec.rb | 2 +- 8 files changed, 27 insertions(+), 80 deletions(-) delete mode 100644 lib/timber/contexts/tags.rb diff --git a/lib/timber/contexts.rb b/lib/timber/contexts.rb index 0c2e7104..11bad8bf 100644 --- a/lib/timber/contexts.rb +++ b/lib/timber/contexts.rb @@ -3,7 +3,6 @@ require "timber/contexts/organization" require "timber/contexts/runtime" require "timber/contexts/system" -require "timber/contexts/tags" require "timber/contexts/user" module Timber diff --git a/lib/timber/contexts/tags.rb b/lib/timber/contexts/tags.rb deleted file mode 100644 index f344c63d..00000000 --- a/lib/timber/contexts/tags.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Timber - module Contexts - # Adds unnamed tags to the context. - # - # **Warning:** It is highly recommend that you use custom contexts instead. As they are - # more descriptive. This module exists primarily to support the ActiveSupport::TaggedLogging - # antipattern. - class Tags < Context - @keyspace = :tags - - attr_reader :values - - def initialize(attributes) - @values = attributes[:values] || raise(ArgumentError.new(":values is required")) - end - - def as_json(_options = {}) - values - end - end - end -end \ No newline at end of file diff --git a/lib/timber/log_entry.rb b/lib/timber/log_entry.rb index d5fb783d..3bf877dd 100644 --- a/lib/timber/log_entry.rb +++ b/lib/timber/log_entry.rb @@ -5,7 +5,7 @@ class LogEntry #:nodoc: DT_PRECISION = 6.freeze SCHEMA = "https://raw.githubusercontent.com/timberio/log-event-json-schema/1.2.2/schema.json".freeze - attr_reader :level, :time, :progname, :message, :context_snapshot, :event + attr_reader :context_snapshot, :event, :level, :message, :progname, :tags, :time # Creates a log entry suitable to be sent to the Timber API. # @param severity [Integer] the log level / severity @@ -17,11 +17,12 @@ class LogEntry #:nodoc: # @param event [Timber.Event] structured data representing the log line event. This should be # an instance of `Timber.Event`. # @return [LogEntry] the resulting LogEntry object - def initialize(level, time, progname, message, context_snapshot, event) + def initialize(level, time, progname, message, context_snapshot, event, tags) @level = level @time = time.utc @progname = progname @message = message + @tags = tags context_snapshot = {} if context_snapshot.nil? system_context = Contexts::System.new(pid: Process.pid) @@ -33,7 +34,7 @@ def initialize(level, time, progname, message, context_snapshot, event) def as_json(options = {}) options ||= {} - hash = {:level => level, :dt => formatted_dt, :message => message} + hash = {:level => level, :dt => formatted_dt, :message => message, :tags => tags} if !event.nil? hash[:event] = event @@ -45,7 +46,7 @@ def as_json(options = {}) hash[:"$schema"] = SCHEMA - if options[:only] + hash = if options[:only] hash.select do |key, _value| options[:only].include?(key) end @@ -56,10 +57,12 @@ def as_json(options = {}) else hash end + + Util::Hash.compact(hash) end def to_json(options = {}) - Util::Hash.compact(as_json(options)).to_json + as_json(options).to_json end def to_msgpack(*args) diff --git a/lib/timber/logger.rb b/lib/timber/logger.rb index 859f4c0f..c5791607 100644 --- a/lib/timber/logger.rb +++ b/lib/timber/logger.rb @@ -76,13 +76,23 @@ class Formatter def build_log_entry(severity, time, progname, msg) level = SEVERITY_MAP.fetch(severity) context_snapshot = CurrentContext.instance.snapshot + tags = extract_active_support_tagged_logging_tags + tags += msg.delete(:tags) if msg.is_a?(Hash) && msg.key?(:tags) event = Events.build(msg) + if event - LogEntry.new(level, time, progname, event.message, context_snapshot, event) + LogEntry.new(level, time, progname, event.message, context_snapshot, event, tags) else - LogEntry.new(level, time, progname, msg, context_snapshot, nil) + LogEntry.new(level, time, progname, msg, context_snapshot, nil, tags) end end + + # Because of all the crazy ways Rails has attempted this we need this crazy method. + def extract_active_support_tagged_logging_tags + Thread.current[:activesupport_tagged_logging_tags] || + Thread.current["activesupport_tagged_logging_tags:#{object_id}"] || + [] + end end # Structures your log messages into Timber's hybrid format, which makes diff --git a/lib/timber/probes/active_support_tagged_logging.rb b/lib/timber/probes/active_support_tagged_logging.rb index d105b741..78749b57 100644 --- a/lib/timber/probes/active_support_tagged_logging.rb +++ b/lib/timber/probes/active_support_tagged_logging.rb @@ -17,26 +17,6 @@ def call(severity, timestamp, progname, msg) super(severity, timestamp, progname, "#{tags_text}#{msg}") end end - - def push_tags(*tags) - _timber_original_push_tags(*tags).tap do - if current_tags.size > 0 - context = Contexts::Tags.new(values: current_tags) - CurrentContext.add(context) - end - end - end - - def pop_tags(size = 1) - _timber_original_pop_tags(size).tap do - if current_tags.size == 0 - CurrentContext.remove(Contexts::Tags) - else - context = Contexts::Tags.new(values: current_tags) - CurrentContext.add(context) - end - end - end end end end @@ -44,29 +24,6 @@ def pop_tags(size = 1) module LoggerMethods def self.included(klass) klass.class_eval do - alias_method :_timber_original_push_tags, :push_tags - alias_method :_timber_original_pop_tags, :pop_tags - - def push_tags(*tags) - _timber_original_push_tags(*tags).tap do - if current_tags.size > 0 - context = Contexts::Tags.new(values: current_tags) - CurrentContext.add(context) - end - end - end - - def pop_tags(size = 1) - _timber_original_pop_tags(size).tap do - if current_tags.size == 0 - CurrentContext.remove(Contexts::Tags) - else - context = Contexts::Tags.new(values: current_tags) - CurrentContext.add(context) - end - end - end - def add(severity, message = nil, progname = nil, &block) if message.nil? if block_given? diff --git a/spec/timber/log_devices/http_spec.rb b/spec/timber/log_devices/http_spec.rb index 64de9697..9837e13f 100644 --- a/spec/timber/log_devices/http_spec.rb +++ b/spec/timber/log_devices/http_spec.rb @@ -58,9 +58,9 @@ it "should add a request to the queue" do http = described_class.new("MYKEY", threads: false) - log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 1", nil, nil) + log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 1", nil, nil, []) http.write(log_entry) - log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 2", nil, nil) + log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 2", nil, nil, []) http.write(log_entry) http.send(:flush) request_queue = http.instance_variable_get(:@request_queue) @@ -109,9 +109,9 @@ to_return(:status => 200, :body => "", :headers => {}) http = described_class.new("MYKEY", flush_interval: 0.1) - log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 1", nil, nil) + log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 1", nil, nil, []) http.write(log_entry) - log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 2", nil, nil) + log_entry = Timber::LogEntry.new("INFO", time, nil, "test log message 2", nil, nil, []) http.write(log_entry) sleep 0.3 diff --git a/spec/timber/log_entry_spec.rb b/spec/timber/log_entry_spec.rb index 8c70a307..9fb1743f 100644 --- a/spec/timber/log_entry_spec.rb +++ b/spec/timber/log_entry_spec.rb @@ -7,9 +7,9 @@ it "should encode properly with an event and context" do event = Timber::Events::Custom.new(type: :event_type, message: "event_message", data: {a: 1}) context = {custom: Timber::Contexts::Custom.new(type: :context_type, data: {b: 1})} - log_entry = described_class.new("INFO", time, nil, "log message", context, event) + log_entry = described_class.new("INFO", time, nil, "log message", context, event, []) msgpack = log_entry.to_msgpack - expect(msgpack).to start_with("\x86\xA5level\xA4INFO\xA2dt\xBB2016-09-01T12:00:00.000000Z\xA7message\xABlog message\xA5event".force_encoding("ASCII-8BIT")) + expect(msgpack).to start_with("\x86\xA5level\xA4INFO\xA2dt\xBB2016-09-01T12:00:00.000000Z".force_encoding("ASCII-8BIT")) end end end \ No newline at end of file diff --git a/spec/timber/logger_spec.rb b/spec/timber/logger_spec.rb index a2ba8a18..634cd22c 100644 --- a/spec/timber/logger_spec.rb +++ b/spec/timber/logger_spec.rb @@ -89,7 +89,7 @@ logger.tagged("tag") do logger.info(message) end - expect(io.string).to include("\"context\":{\"tags\":[\"tag\"]") + expect(io.string).to include("\"tags\":[\"tag\"]") end end end