Skip to content

Commit

Permalink
Merge pull request #1361 from appsignal/fix-double-formatting
Browse files Browse the repository at this point in the history
Set formatter on all broadcasted loggers
  • Loading branch information
unflxw authored Dec 31, 2024
2 parents e23c26f + cd2f9fb commit 8b5c740
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 27 deletions.
6 changes: 6 additions & 0 deletions .changesets/fix-double-newlines-in-broadcasted-loggers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: fix
---

Fix an issue where loggers, when broadcasted to by `Appsignal::Logger#broadcast_to`, would format again messages that have already been formatted by the broadcaster, causing the resulting message emitted by the logger to contain double newlines.
12 changes: 10 additions & 2 deletions lib/appsignal/logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ def initialize(group, level: INFO, format: PLAINTEXT, attributes: {})
@loggers = []
end

# When a formatter is set on the logger (e.g. when wrapping the logger in
# `ActiveSupport::TaggedLogging`) we want to set that formatter on all the
# loggers that are being broadcasted to.
def formatter=(formatter)
super
@loggers.each { |logger| logger.formatter = formatter }
end

# We support the various methods in the Ruby
# logger class by supplying this method.
# @api private
Expand All @@ -59,8 +67,6 @@ def add(severity, message = nil, group = nil)
end
return if message.nil?

message = formatter.call(severity, Time.now, group, message) if formatter

@loggers.each do |logger|
logger.add(severity, message, group)
rescue
Expand All @@ -74,6 +80,8 @@ def add(severity, message = nil, group = nil)
return
end

message = formatter.call(severity, Time.now, group, message) if formatter

Appsignal::Extension.log(
group,
SEVERITY_MAP.fetch(severity, 0),
Expand Down
42 changes: 17 additions & 25 deletions spec/lib/appsignal/logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] Some message"),
"[My tag] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -20,9 +20,7 @@
"group",
3,
0,
a_string_starting_with(
"[My tag] [My other tag] [Nested tag] [Nested other tag] Some message"
),
"[My tag] [My other tag] [Nested tag] [Nested other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -44,9 +42,7 @@
"group",
3,
0,
a_string_starting_with(
"[Request tag] [Second tag] [First message] [My other tag] Some message"
),
"[Request tag] [Second tag] [First message] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -58,7 +54,7 @@
"group",
3,
0,
a_string_starting_with("[Request tag] [Second tag] [Second message] Some message"),
"[Request tag] [Second tag] [Second message] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -71,7 +67,7 @@
"group",
3,
0,
a_string_starting_with("[Third message] Some message"),
"[Third message] Some message\n",
Appsignal::Utils::Data.generate({})
)
end
Expand All @@ -87,9 +83,7 @@
"group",
3,
0,
a_string_starting_with(
"[Request tag] [Second tag] [First message] [My other tag] Some message"
),
"[Request tag] [Second tag] [First message] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)
end
Expand All @@ -105,9 +99,7 @@
"group",
3,
0,
a_string_starting_with(
"[Request tag] [Second tag] [First message] [My other tag] Some message"
),
"[Request tag] [Second tag] [First message] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -118,7 +110,7 @@
"group",
3,
0,
a_string_starting_with("[First message] [My other tag] Some message"),
"[First message] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)
end
Expand All @@ -129,7 +121,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] Some message"),
"[My tag] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -150,7 +142,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] Some message"),
"[My tag] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -163,7 +155,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] Some message"),
"[My tag] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -175,7 +167,7 @@
"group",
3,
0,
a_string_starting_with("Some message"),
"Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -188,7 +180,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] [My third tag] Some message"),
"[My tag] [My other tag] [My third tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -201,7 +193,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] [My third tag] Some message"),
"[My tag] [My other tag] [My third tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -219,7 +211,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] [My third tag] Some message"),
"[My tag] [My other tag] [My third tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand Down Expand Up @@ -407,7 +399,7 @@
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] Some message"),
"[My tag] [My other tag] Some message\n",
Appsignal::Utils::Data.generate({})
)

Expand All @@ -416,7 +408,7 @@
end

expect(other_stream.string)
.to include("INFO -- group: [My tag] [My other tag] Some message")
.to eq("[My tag] [My other tag] Some message\n")
end
end
end
Expand Down

0 comments on commit 8b5c740

Please sign in to comment.