Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log Redis command arguments if send_default_pii = true #1726

Merged
merged 4 commits into from
Mar 4, 2022
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Features

- Log Redis command arguments when sending PII is enabled [#1726](https://github.com/getsentry/sentry-ruby/pull/1726)
- Add request env to sampling context [#1749](https://github.com/getsentry/sentry-ruby/pull/1749)

### Bug Fixes
Expand Down
10 changes: 6 additions & 4 deletions sentry-ruby/lib/sentry/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
module Sentry
# @api private
class Redis
OP_NAME ||= "db.redis.command"
LOGGER_NAME ||= :redis_logger
OP_NAME = "db.redis.command"
LOGGER_NAME = :redis_logger

def initialize(commands, host, port, db)
@commands, @host, @port, @db = commands, host, port, db
Expand Down Expand Up @@ -60,9 +60,11 @@ def commands_description

def parsed_commands
commands.map do |statement|
command, key, *_values = statement
command, key, *arguments = statement

{ command: command.to_s.upcase, key: key }
{ command: command.to_s.upcase, key: key }.tap do |command_set|
command_set[:arguments] = arguments.join(" ") if Sentry.configuration.send_default_pii
end
end
end

Expand Down
50 changes: 42 additions & 8 deletions sentry-ruby/spec/sentry/breadcrumb/redis_logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,48 @@
end
end

it "adds Redis command breadcrumb with command and key" do
result = redis.set("key", "value")

expect(result).to eq("OK")
expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
category: "db.redis.command",
data: { commands: [{ command: "SET", key: "key" }], server: "127.0.0.1:6379/0" }
)
context "config.send_default_pii = false" do
before { Sentry.configuration.send_default_pii = false }

it "adds Redis command breadcrumb with command and key" do
result = redis.set("key", "value")

expect(result).to eq("OK")
expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
category: "db.redis.command",
data: { commands: [{ command: "SET", key: "key" }], server: "127.0.0.1:6379/0" }
)
end
end

context "config.send_default_pii = true" do
before { Sentry.configuration.send_default_pii = true }

it "adds Redis command breadcrumb with command, key and arguments" do
result = redis.set("key", "value")

expect(result).to eq("OK")
expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
category: "db.redis.command",
data: { commands: [{ command: "SET", key: "key", arguments: "value" }], server: "127.0.0.1:6379/0" }
)
end

it "logs complex Redis commands with multiple arguments" do
redis.hmset("hashkey", "key1", "value1", "key2", "value2")

expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
data: include(commands: [{ command: "HMSET", key: "hashkey", arguments: "key1 value1 key2 value2" }])
)
end

it "logs Redis command with options" do
redis.zrangebyscore("sortedsetkey", "1", "2", with_scores: true, limit: [0, 10])

expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
data: include(commands: [{ command: "ZRANGEBYSCORE", key: "sortedsetkey", arguments: "1 2 WITHSCORES LIMIT 0 10" }])
)
end
end

context "calling Redis command which doesn't require a key" do
Expand Down
46 changes: 33 additions & 13 deletions sentry-ruby/spec/sentry/redis_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,41 @@
end
end

context "calling Redis SET command" do
let(:result) { redis.set("key", "value") }
context "config.send_default_pii = true" do
before { Sentry.configuration.send_default_pii = true }

it "records the Redis call's span with command and key" do
transaction = Sentry.start_transaction
Sentry.get_current_scope.set_span(transaction)
context "calling Redis SET command" do
let(:result) { redis.set("key", "value") }

expect(result).to eq("OK")
request_span = transaction.span_recorder.spans.last
expect(request_span.op).to eq("db.redis.command")
expect(request_span.start_timestamp).not_to be_nil
expect(request_span.timestamp).not_to be_nil
expect(request_span.start_timestamp).not_to eq(request_span.timestamp)
expect(request_span.description).to eq("SET key")
expect(request_span.data).to eq({ server: "127.0.0.1:6379/0" })
it "records the Redis call's span with command and key" do
transaction = Sentry.start_transaction
Sentry.get_current_scope.set_span(transaction)

expect(result).to eq("OK")
request_span = transaction.span_recorder.spans.last
expect(request_span.op).to eq("db.redis.command")
expect(request_span.start_timestamp).not_to be_nil
expect(request_span.timestamp).not_to be_nil
expect(request_span.start_timestamp).not_to eq(request_span.timestamp)
expect(request_span.description).to eq("SET key value")
expect(request_span.data).to eq({ server: "127.0.0.1:6379/0" })
end
end
end

context "config.send_default_pii = false" do
before { Sentry.configuration.send_default_pii = false }

context "calling Redis SET command" do
let(:result) { redis.set("key", "value") }

it "records the Redis call's span with command and key" do
transaction = Sentry.start_transaction
Sentry.get_current_scope.set_span(transaction)

expect(result).to eq("OK")
expect(transaction.span_recorder.spans.last.description).to eq("SET key")
end
end
end

Expand Down