Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 14 additions & 13 deletions sentry-ruby/lib/sentry/structured_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ def initialize(config)
# @param attributes [Hash] Additional attributes to include with the log
#
# @return [LogEvent, nil] The created log event or nil if logging is disabled
def trace(message, parameters = [], **attributes)
log(__method__, message, parameters: parameters, **attributes)
def trace(message = nil, parameters = [], **attributes, &block)
log(__method__, message, parameters: parameters, **attributes, &block)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

DebugStructuredLogger not updated for block syntax support

Medium Severity

DebugStructuredLogger overrides all the same log-level methods and log with the old signatures — message is required and no &block is accepted. After this change, calling block syntax like logger.info { "Hello" } on a DebugStructuredLogger instance raises an ArgumentError because it still expects a mandatory message argument and silently ignores the block. This breaks the compatibility goal for anyone using the debug logger variant.

Additional Locations (1)
Fix in Cursor Fix in Web

end

# Logs a message at DEBUG level
Expand All @@ -70,8 +70,8 @@ def trace(message, parameters = [], **attributes)
# @param attributes [Hash] Additional attributes to include with the log
#
# @return [LogEvent, nil] The created log event or nil if logging is disabled
def debug(message, parameters = [], **attributes)
log(__method__, message, parameters: parameters, **attributes)
def debug(message = nil, parameters = [], **attributes, &block)
log(__method__, message, parameters: parameters, **attributes, &block)
end

# Logs a message at INFO level
Expand All @@ -81,8 +81,8 @@ def debug(message, parameters = [], **attributes)
# @param attributes [Hash] Additional attributes to include with the log
#
# @return [LogEvent, nil] The created log event or nil if logging is disabled
def info(message, parameters = [], **attributes)
log(__method__, message, parameters: parameters, **attributes)
def info(message = nil, parameters = [], **attributes, &block)
log(__method__, message, parameters: parameters, **attributes, &block)
end

# Logs a message at WARN level
Expand All @@ -92,8 +92,8 @@ def info(message, parameters = [], **attributes)
# @param attributes [Hash] Additional attributes to include with the log
#
# @return [LogEvent, nil] The created log event or nil if logging is disabled
def warn(message, parameters = [], **attributes)
log(__method__, message, parameters: parameters, **attributes)
def warn(message = nil, parameters = [], **attributes, &block)
log(__method__, message, parameters: parameters, **attributes, &block)
end

# Logs a message at ERROR level
Expand All @@ -103,8 +103,8 @@ def warn(message, parameters = [], **attributes)
# @param attributes [Hash] Additional attributes to include with the log
#
# @return [LogEvent, nil] The created log event or nil if logging is disabled
def error(message, parameters = [], **attributes)
log(__method__, message, parameters: parameters, **attributes)
def error(message = nil, parameters = [], **attributes, &block)
log(__method__, message, parameters: parameters, **attributes, &block)
end

# Logs a message at FATAL level
Expand All @@ -114,8 +114,8 @@ def error(message, parameters = [], **attributes)
# @param attributes [Hash] Additional attributes to include with the log
#
# @return [LogEvent, nil] The created log event or nil if logging is disabled
def fatal(message, parameters = [], **attributes)
log(__method__, message, parameters: parameters, **attributes)
def fatal(message = nil, parameters = [], **attributes, &block)
log(__method__, message, parameters: parameters, **attributes, &block)
end

# Logs a message at the specified level
Expand All @@ -126,7 +126,8 @@ def fatal(message, parameters = [], **attributes)
# @param attributes [Hash] Additional attributes to include with the log
#
# @return [LogEvent, nil] The created log event or nil if logging is disabled
def log(level, message, parameters:, **attributes)
def log(level, message = nil, parameters:, **attributes, &block)
message = yield if message.nil? && block_given?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nil message causes NoMethodError in LogEvent

Low Severity

Making message optional without a nil guard means calling a log method with no arguments and no block passes nil through to LogEvent#is_template?, which calls body.include?("%s") on nil, raising a NoMethodError. Previously, this scenario raised a clear ArgumentError at the method boundary.

Fix in Cursor Fix in Web

case parameters
when Array then
Sentry.capture_log(message, level: level, severity: LEVELS[level], parameters: parameters, **attributes)
Comment on lines 126 to 133
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: DebugStructuredLogger methods were not updated to support block syntax. Calling them with a block will raise an ArgumentError because the message parameter is still required.
Severity: CRITICAL

Suggested Fix

Update the method definitions in DebugStructuredLogger to match StructuredLogger. Make the message parameter optional with a default nil value, add a &block parameter, and evaluate the block if no message is provided. This will ensure both logger classes behave consistently.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: sentry-ruby/lib/sentry/structured_logger.rb#L126-L133

Potential issue: The pull request adds block syntax support to `StructuredLogger`'s
logging methods, but the corresponding methods in `DebugStructuredLogger` were not
updated. The `DebugStructuredLogger` methods do not accept a block parameter (`&block`)
and still require a non-optional `message` argument. Consequently, if
`config.structured_logging.logger_class` is set to `Sentry::DebugStructuredLogger`, any
call to the logger using block syntax, such as `Sentry.logger.info { "message" }`, will
raise an `ArgumentError` and crash the application. This breaks the intended
interchangeability with Ruby's standard logger.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down
11 changes: 11 additions & 0 deletions sentry-ruby/spec/sentry/structured_logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,17 @@
expect(log_event[:attributes]["sentry.message.parameter.day"]).to eql({ value: "Monday", type: "string" })
end

it "logs with block syntax for compatibility with ruby Logger" do
Sentry.logger.public_send(level) { "Hello world" }

expect(sentry_logs).to_not be_empty

log_event = sentry_logs.last

expect(log_event[:level]).to eql(level)
expect(log_event[:body]).to eql("Hello world")
end

context "handling of malformed strings" do
let(:malformed_string_default) do
Sentry::Utils::EncodingHelper::MALFORMED_STRING
Expand Down
Loading