Skip to content

feat: Replace obfuscate setting with an exclude attribute setting #2346

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.

Sign up for GitHub

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

Open
thompson-tomo wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −127
Open

feat: Replace obfuscate setting with an exclude attribute setting #2346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion instrumentation/trilogy/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ This gem requires Trilogy 2.11 or higher and will not work with a future Trilogy

| Option | Default | Description |
| ------ | ------- | ----------- |
| `db_statement` | `:obfuscate` | Controls how SQL queries appear in spans. `:obfuscate` replaces literal values with `?`, `:include` records the raw SQL, `:omit` excludes the attribute entirely. |
| `db_statement` | `:obfuscate` | Deprecated, use `exclude_dbquerytext` on stable/dup. Controls how SQL queries appear in spans. `:obfuscate` replaces literal values with `?`, `:include` records the raw SQL, `:omit` excludes the attribute entirely. |
| `exclude_dbquerytext` | false | Defines if the db.query.text attribute should be included. |
| `obfuscation_limit` | `2000` | Maximum length of the obfuscated SQL statement. Statements exceeding this limit are truncated. |
| `peer_service` | `nil` | Deprecated with no replacement. Sets the `peer.service` attribute on spans (old semantic conventions only). |
| `propagator` | `'none'` | Propagator for injecting trace context into SQL comments. `'none'` disables propagation, `'tracecontext'` uses W3C Trace Context, `'vitess'` uses Vitess-style propagation (requires `opentelemetry-propagator-vitess` gem). |
Expand Down
1 change: 1 addition & 0 deletions instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/instrumentation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
option :obfuscation_limit, default: 2000, validate: :integer
option :propagator, default: 'none', validate: %w[none tracecontext vitess]
option :record_exception, default: true, validate: :boolean
option :exclude_dbquerytext, default: false, validate: :boolean

attr_reader :propagator, :semconv

Expand Down
20 changes: 6 additions & 14 deletions instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/dup/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,20 +114,12 @@ def client_attributes(sql = nil)
# Old convention
attributes['db.instance.id'] = @connected_host unless @connected_host.nil?

if sql
case config[:db_statement]
when :obfuscate
obfuscated = OpenTelemetry::Helpers::SqlProcessor.obfuscate_sql(sql, obfuscation_limit: config[:obfuscation_limit], adapter: :mysql)
# Old convention
attributes[::OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT] = obfuscated
# Stable convention
attributes['db.query.text'] = obfuscated
when :include
# Old convention
attributes[::OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT] = sql
# Stable convention
attributes['db.query.text'] = sql
end
if sql && !config[:exclude_dbquerytext]
obfuscated = OpenTelemetry::Helpers::SqlProcessor.obfuscate_sql(sql, obfuscation_limit: config[:obfuscation_limit], adapter: :mysql)
# Old convention
attributes[::OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT] = obfuscated
# Stable convention
attributes['db.query.text'] = obfuscated
end

attributes
Expand Down
11 changes: 3 additions & 8 deletions instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/stable/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,9 @@ def _build_otel_base_attributes
def client_attributes(sql = nil)
attributes = @_otel_base_attributes.dup

if sql
case config[:db_statement]
when :obfuscate
attributes['db.query.text'] =
OpenTelemetry::Helpers::SqlProcessor.obfuscate_sql(sql, obfuscation_limit: config[:obfuscation_limit], adapter: :mysql)
when :include
attributes['db.query.text'] = sql
end
if sql && !config[:exclude_dbquerytext]
attributes['db.query.text'] =
OpenTelemetry::Helpers::SqlProcessor.obfuscate_sql(sql, obfuscation_limit: config[:obfuscation_limit], adapter: :mysql)
end

attributes
Expand Down
37 changes: 8 additions & 29 deletions .../trilogy/test/opentelemetry/instrumentation/trilogy/patches/dup/client_attributes_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def build_test_client(options)
exporter.reset
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install({
db_statement: :omit,
exclude_dbquerytext: true,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand Down Expand Up @@ -152,14 +152,14 @@ def build_test_client(options)
refute b.key?('extra')
end

describe 'with sql and db_statement config' do
describe 'with sql and exclude_dbquerytext config' do
before do
instrumentation.instance_variable_set(:@installed, false)
end

it 'includes SQL in db.statement (old) when db_statement is :include' do
it 'includes SQL in db.statement (old) when exclude_dbquerytext is false' do
instrumentation.install({
db_statement: :include,
exclude_dbquerytext: false,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand All @@ -170,9 +170,9 @@ def build_test_client(options)
assert_equal 'SELECT * FROM users', attrs[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]
end

it 'includes SQL in db.query.text (stable) when db_statement is :include' do
it 'includes SQL in db.query.text (stable) when exclude_dbquerytext is false' do
instrumentation.install({
db_statement: :include,
exclude_dbquerytext: false,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand All @@ -183,9 +183,9 @@ def build_test_client(options)
assert_equal 'SELECT * FROM users', attrs['db.query.text']
end

it 'omits both db.statement and db.query.text when db_statement is :omit' do
it 'omits both db.statement and db.query.text when exclude_dbquerytext is true' do
instrumentation.install({
db_statement: :omit,
exclude_dbquerytext: true,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand All @@ -196,27 +196,6 @@ def build_test_client(options)
refute attrs.key?(OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT)
refute attrs.key?('db.query.text')
end

it 'obfuscates SQL in both attributes when db_statement is :obfuscate' do
instrumentation.install({
db_statement: :obfuscate,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
obfuscation_limit: 2000,
peer_service: nil
})
attrs = client.send(:client_attributes, 'SELECT * FROM users WHERE id = 1')

old_stmt = attrs[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]
new_stmt = attrs['db.query.text']

assert old_stmt, 'expected db.statement to be present'
assert new_stmt, 'expected db.query.text to be present'
refute_includes old_stmt, '1'
refute_includes new_stmt, '1'
assert_equal old_stmt, new_stmt
end
end
end
end
41 changes: 13 additions & 28 deletions ...on/trilogy/test/opentelemetry/instrumentation/trilogy/patches/dup/instrumentation_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
end

describe 'with default options' do
it 'obfuscates sql in both old and stable attributes' do
it 'removes sql in both old and stable attributes' do
client.query('SELECT 1')

_(span.name).must_equal 'select'
Expand Down Expand Up @@ -405,23 +405,8 @@
end
end

describe 'when db_statement is set to include' do
let(:config) { { db_statement: :include } }

it 'includes the db query statement in both attributes' do
sql = 'SELECT * from users where users.id = 1 and users.email = "test@test.com"'
expect do
client.query(sql)
end.must_raise Trilogy::Error

_(span.name).must_equal 'select'
_(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal sql
_(span.attributes['db.query.text']).must_equal sql
end
end

describe 'when db_statement is set to obfuscate' do
let(:config) { { db_statement: :obfuscate } }
describe 'when exclude_dbquerytext is set to false' do
let(:config) { { exclude_dbquerytext: false } }

it 'obfuscates SQL parameters in both attributes' do
sql = 'SELECT * from users where users.id = 1 and users.email = "test@test.com"'
Expand Down Expand Up @@ -450,7 +435,7 @@
end

describe 'with obfuscation_limit' do
let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }
let(:config) { { exclude_dbquerytext: false, obfuscation_limit: 10 } }

it 'returns a message when the limit is reached' do
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
Expand Down Expand Up @@ -592,8 +577,8 @@
end
end

describe 'when db_statement is set to omit' do
let(:config) { { db_statement: :omit } }
describe 'when exclude_dbquerytext is set to true' do
let(:config) { { exclude_dbquerytext: true } }

it 'does not include SQL statement in either attribute' do
sql = 'SELECT * from users where users.id = 1 and users.email = "test@test.com"'
Expand All @@ -607,10 +592,10 @@
end
end

describe 'when db_statement is configured via environment variable' do
describe 'when db_statement set as omit' do
describe 'when dbquerytext is configured via environment variable' do
describe 'when exclude_dbquerytext set as true' do
it 'omits both db.statement and db.query.text attributes' do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'db_statement=omit;') do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'exclude_dbquerytext=true;') do
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
Expand All @@ -627,9 +612,9 @@
end
end

describe 'when db_statement set as obfuscate' do
describe 'when exclude_dbquerytext set as false' do
it 'obfuscates SQL parameters in both attributes' do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'db_statement=obfuscate;') do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'exclude_dbquerytext=false;') do
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install

Expand All @@ -649,10 +634,10 @@
end

describe 'when db_statement is set differently than local config' do
let(:config) { { db_statement: :omit } }
let(:config) { { exclude_dbquerytext: true } }

it 'overrides local config and obfuscates SQL parameters in both attributes' do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'db_statement=obfuscate') do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'exclude_dbquerytext=false') do
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install

Expand Down
31 changes: 8 additions & 23 deletions ...ilogy/test/opentelemetry/instrumentation/trilogy/patches/stable/client_attributes_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def build_test_client(options)
exporter.reset
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install({
db_statement: :omit,
exclude_dbquerytext: true,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand Down Expand Up @@ -124,14 +124,14 @@ def build_test_client(options)
end
end

describe 'with sql and db_statement config' do
describe 'with sql and exclude_dbquerytext config' do
before do
instrumentation.instance_variable_set(:@installed, false)
end

it 'includes SQL as db.query.text when db_statement is :include' do
it 'includes SQL as db.query.text when exclude_dbquerytext is false' do
instrumentation.install({
db_statement: :include,
exclude_dbquerytext: false,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand All @@ -142,9 +142,9 @@ def build_test_client(options)
assert_equal 'SELECT * FROM users', attrs['db.query.text']
end

it 'does not include db.statement when db_statement is :include' do
it 'does not include db.statement when exclude_dbquerytext is false' do
instrumentation.install({
db_statement: :include,
exclude_dbquerytext: false,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand All @@ -155,9 +155,9 @@ def build_test_client(options)
refute attrs.key?(OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT)
end

it 'omits db.query.text when db_statement is :omit' do
it 'omits db.query.text when exclude_dbquerytext is true' do
instrumentation.install({
db_statement: :omit,
exclude_dbquerytext: true,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
Expand All @@ -167,21 +167,6 @@ def build_test_client(options)
attrs = client.send(:client_attributes, 'SELECT * FROM users')
refute attrs.key?('db.query.text')
end

it 'obfuscates SQL in db.query.text when db_statement is :obfuscate' do
instrumentation.install({
db_statement: :obfuscate,
span_name: :statement_type,
propagator: 'none',
record_exception: true,
obfuscation_limit: 2000,
peer_service: nil
})
attrs = client.send(:client_attributes, 'SELECT * FROM users WHERE id = 1')
stmt = attrs['db.query.text']
assert stmt, 'expected db.query.text to be present'
refute_includes stmt, '1'
end
end
end
end
34 changes: 10 additions & 24 deletions ...trilogy/test/opentelemetry/instrumentation/trilogy/patches/stable/instrumentation_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
end

describe 'with default options' do
it 'obfuscates sql' do
it 'drops sql' do
client.query('SELECT 1')

_(span.name).must_equal 'select'
Expand Down Expand Up @@ -304,22 +304,8 @@
end
end

describe 'when db_statement is set to include' do
let(:config) { { db_statement: :include } }

it 'includes the db query statement' do
sql = 'SELECT * from users where users.id = 1 and users.email = "test@test.com"'
expect do
client.query(sql)
end.must_raise Trilogy::Error

_(span.name).must_equal 'select'
_(span.attributes['db.query.text']).must_equal sql
end
end

describe 'when db_statement is set to obfuscate' do
let(:config) { { db_statement: :obfuscate } }
describe 'when exclude_dbquerytext is set to false' do
let(:config) { { exclude_dbquerytext: false } }

it 'obfuscates SQL parameters in db.query.text' do
sql = 'SELECT * from users where users.id = 1 and users.email = "test@test.com"'
Expand All @@ -346,7 +332,7 @@
end

describe 'with obfuscation_limit' do
let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }
let(:config) { { exclude_dbquerytext: false, obfuscation_limit: 10 } }

it 'returns a message when the limit is reached' do
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
Expand Down Expand Up @@ -487,8 +473,8 @@
end
end

describe 'when db_statement is set to omit' do
let(:config) { { db_statement: :omit } }
describe 'when exclude_dbquerytext is set to true' do
let(:config) { { exclude_dbquerytext: true } }

it 'does not include SQL statement as db.query.text attribute' do
sql = 'SELECT * from users where users.id = 1 and users.email = "test@test.com"'
Expand All @@ -504,7 +490,7 @@
describe 'when db_statement is configured via environment variable' do
describe 'when db_statement set as omit' do
it 'omits db.query.text attribute' do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'db_statement=omit;') do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'exclude_dbquerytext=true;') do
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
Expand All @@ -521,7 +507,7 @@

describe 'when db_statement set as obfuscate' do
it 'obfuscates SQL parameters in db.query.text' do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'db_statement=obfuscate;') do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'exclude_dbquerytext=false;') do
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install

Expand All @@ -539,10 +525,10 @@
end

describe 'when db_statement is set differently than local config' do
let(:config) { { db_statement: :omit } }
let(:config) { { exclude_dbquerytext: true } }

it 'overrides local config and obfuscates SQL parameters in db.query.text' do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'db_statement=obfuscate') do
OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_TRILOGY_CONFIG_OPTS' => 'exclude_dbquerytext=false') do
instrumentation.instance_variable_set(:@installed, false)
instrumentation.install

Expand Down
Loading