From 279c669d05323ba502e057da0f07592a8dac82d0 Mon Sep 17 00:00:00 2001 From: Reid Lynch Date: Fri, 23 Dec 2022 10:39:29 -0500 Subject: [PATCH 1/3] feat: add config[:obfuscation_limit] to pg and mysql2 --- .../instrumentation/mysql2/instrumentation.rb | 1 + .../instrumentation/mysql2/patches/client.rb | 6 ++++-- .../instrumentation/mysql2/instrumentation_test.rb | 14 ++++++++++++++ instrumentation/pg/example/pg.rb | 2 +- .../instrumentation/pg/instrumentation.rb | 1 + .../instrumentation/pg/patches/connection.rb | 6 ++++-- .../instrumentation/pg/instrumentation_test.rb | 14 ++++++++++++++ 7 files changed, 39 insertions(+), 5 deletions(-) diff --git a/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/instrumentation.rb b/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/instrumentation.rb index 6bb5d4013b..adea0a4b8d 100644 --- a/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/instrumentation.rb +++ b/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/instrumentation.rb @@ -31,6 +31,7 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base option :enable_sql_obfuscation, default: false, validate: :boolean option :db_statement, default: :include, validate: %I[omit include obfuscate] option :span_name, default: :statement_type, validate: %I[statement_type db_name db_operation_and_name] + option :obfuscation_limit, default: 2000, validate: :integer private diff --git a/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb b/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb index 1b09fbbcc8..a5b2d42498 100644 --- a/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb +++ b/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb @@ -30,6 +30,7 @@ module Client # rubocop:disable Metrics/ModuleLength QUERY_NAME_RE = Regexp.new("^(#{QUERY_NAMES.join('|')})", Regexp::IGNORECASE) + # From: https://github.com/newrelic/newrelic-ruby-agent/blob/0235b288d85b8bc795bdc1a24621dd9f84cfef45/lib/new_relic/agent/database/obfuscation_helpers.rb#L9-L34 COMPONENTS_REGEX_MAP = { single_quotes: /'(?:[^']|'')*?(?:\\'.*|'(?!'))/, double_quotes: /"(?:[^"]|"")*?(?:\\".*|"(?!"))/, @@ -70,8 +71,9 @@ def query(sql, options = {}) private def obfuscate_sql(sql) - if sql.size > 2000 - 'SQL query too large to remove sensitive data ...' + if sql.size > config[:obfuscation_limit] + truncated_sql = sql[..sql.index(generated_mysql_regex) - 1] + truncated_sql + "\nSQL truncated (> #{config[:obfuscation_limit]} characters)" else obfuscated = OpenTelemetry::Common::Utilities.utf8_encode(sql, binary: true) obfuscated = obfuscated.gsub(generated_mysql_regex, '?') diff --git a/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb b/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb index e9c87745b3..9442a8f4e7 100644 --- a/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb +++ b/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb @@ -172,6 +172,20 @@ _(span.attributes['net.peer.name']).must_equal host.to_s _(span.attributes['net.peer.port']).must_equal port.to_s end + + describe 'with obfuscation_limit' do + let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } } + + it 'truncates SQL using config limit' do + sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'" + obfuscated_sql = "SELECT * from users where users.id = \nSQL truncated (> #{config[:obfuscation_limit]} characters)" + expect do + client.query(sql) + end.must_raise Mysql2::Error + + _(span.attributes['db.statement']).must_equal obfuscated_sql + end + end end describe 'when db_statement set as omit' do diff --git a/instrumentation/pg/example/pg.rb b/instrumentation/pg/example/pg.rb index 893d9f9618..1a10898bdf 100644 --- a/instrumentation/pg/example/pg.rb +++ b/instrumentation/pg/example/pg.rb @@ -7,7 +7,7 @@ ENV['OTEL_TRACES_EXPORTER'] = 'console' OpenTelemetry::SDK.configure do |c| - c.use 'OpenTelemetry::Instrumentation::PG', enable_sql_obfuscation: true + c.use 'OpenTelemetry::Instrumentation::PG', db_statement: :obfuscate end conn = PG::Connection.open( diff --git a/instrumentation/pg/lib/opentelemetry/instrumentation/pg/instrumentation.rb b/instrumentation/pg/lib/opentelemetry/instrumentation/pg/instrumentation.rb index 528a3b3602..51b8d85ab9 100644 --- a/instrumentation/pg/lib/opentelemetry/instrumentation/pg/instrumentation.rb +++ b/instrumentation/pg/lib/opentelemetry/instrumentation/pg/instrumentation.rb @@ -44,6 +44,7 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base option :enable_sql_obfuscation, default: false, validate: :boolean option :enable_statement_attribute, default: true, validate: :boolean option :db_statement, default: :include, validate: %I[omit include obfuscate] + option :obfuscation_limit, default: 2000, validate: :integer private diff --git a/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb b/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb index 532ed3b7f8..476a112d00 100644 --- a/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb +++ b/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb @@ -107,8 +107,10 @@ def validated_operation(operation) def obfuscate_sql(sql) return sql unless config[:db_statement] == :obfuscate - # Borrowed from opentelemetry-instrumentation-mysql2 - return 'SQL query too large to remove sensitive data ...' if sql.size > 2000 + if sql.size > config[:obfuscation_limit] + truncated_sql = sql[..sql.index(generated_postgres_regex) - 1] + return truncated_sql + "\nSQL truncated (> #{config[:obfuscation_limit]} characters)" + end # From: # https://github.com/newrelic/newrelic-ruby-agent/blob/9787095d4b5b2d8fcaf2fdbd964ed07c731a8b6b/lib/new_relic/agent/database/obfuscator.rb diff --git a/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb b/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb index 3f4df80d4c..bb960d6f0a 100644 --- a/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb +++ b/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb @@ -269,6 +269,20 @@ _(span.attributes['net.peer.name']).must_equal host.to_s _(span.attributes['net.peer.port']).must_equal port.to_i end + + describe 'with obfuscation_limit' do + let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } } + + it 'truncates SQL using config limit' do + sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'" + obfuscated_sql = "SELECT * from users where users.id = \nSQL truncated (> #{config[:obfuscation_limit]} characters)" + expect do + client.exec(sql) + end.must_raise PG::UndefinedTable + + _(span.attributes['db.statement']).must_equal obfuscated_sql + end + end end describe 'when db_statement is omit' do From e79fd951f136f74bf3e25f95aa02b20e1379ed16 Mon Sep 17 00:00:00 2001 From: Reid Lynch Date: Fri, 23 Dec 2022 10:44:30 -0500 Subject: [PATCH 2/3] add ellipsis --- .../lib/opentelemetry/instrumentation/mysql2/patches/client.rb | 2 +- .../instrumentation/mysql2/instrumentation_test.rb | 2 +- .../lib/opentelemetry/instrumentation/pg/patches/connection.rb | 2 +- .../opentelemetry/instrumentation/pg/instrumentation_test.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb b/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb index a5b2d42498..4240813c31 100644 --- a/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb +++ b/instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb @@ -73,7 +73,7 @@ def query(sql, options = {}) def obfuscate_sql(sql) if sql.size > config[:obfuscation_limit] truncated_sql = sql[..sql.index(generated_mysql_regex) - 1] - truncated_sql + "\nSQL truncated (> #{config[:obfuscation_limit]} characters)" + truncated_sql + "...\nSQL truncated (> #{config[:obfuscation_limit]} characters)" else obfuscated = OpenTelemetry::Common::Utilities.utf8_encode(sql, binary: true) obfuscated = obfuscated.gsub(generated_mysql_regex, '?') diff --git a/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb b/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb index 9442a8f4e7..dfb5d66d31 100644 --- a/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb +++ b/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb @@ -178,7 +178,7 @@ it 'truncates SQL using config limit' do sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'" - obfuscated_sql = "SELECT * from users where users.id = \nSQL truncated (> #{config[:obfuscation_limit]} characters)" + obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)" expect do client.query(sql) end.must_raise Mysql2::Error diff --git a/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb b/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb index 476a112d00..bc82bd939b 100644 --- a/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb +++ b/instrumentation/pg/lib/opentelemetry/instrumentation/pg/patches/connection.rb @@ -109,7 +109,7 @@ def obfuscate_sql(sql) if sql.size > config[:obfuscation_limit] truncated_sql = sql[..sql.index(generated_postgres_regex) - 1] - return truncated_sql + "\nSQL truncated (> #{config[:obfuscation_limit]} characters)" + return truncated_sql + "...\nSQL truncated (> #{config[:obfuscation_limit]} characters)" end # From: diff --git a/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb b/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb index bb960d6f0a..ff52b8e350 100644 --- a/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb +++ b/instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb @@ -275,7 +275,7 @@ it 'truncates SQL using config limit' do sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'" - obfuscated_sql = "SELECT * from users where users.id = \nSQL truncated (> #{config[:obfuscation_limit]} characters)" + obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)" expect do client.exec(sql) end.must_raise PG::UndefinedTable From e9e68db4f2601e13ffab6cf9ca306eaeda92f91c Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Thu, 25 May 2023 06:52:15 -0500 Subject: [PATCH 3/3] fix: remove whitespace --- .../opentelemetry/instrumentation/mysql2/instrumentation_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb b/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb index 4136754260..cd46650823 100644 --- a/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb +++ b/instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb @@ -172,7 +172,6 @@ _(span.attributes['net.peer.name']).must_equal host.to_s _(span.attributes['net.peer.port']).must_equal port.to_s end - it 'encodes invalid byte sequences for db.statement' do # \255 is off-limits https://en.wikipedia.org/wiki/UTF-8#Codepage_layout sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com\255'"