From b6e760d091bf6087100648781c3c2db6df601d06 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Wed, 31 May 2023 19:30:00 +0000 Subject: [PATCH 1/5] feat!: Separate logical MySQL host from connected MySQL server instances have a logical host name and an instance node names. This is particularly useful when running in clustered mode where a logical hostname is used to connect to a cluster but you also want to record what specific node is executing your query. Trilogy exposes the instance node name via `@connected_host` as well as the configured `host` name in its `connection_options`. This breaking change ensures that the `net.peer.name` is always set to the logical host name, while the individual instance name is being recorded in a separate attribute. --- .../instrumentation/trilogy/patches/client.rb | 18 ++---------------- .../trilogy/instrumentation_test.rb | 11 +++++------ 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb b/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb index a845349032..71300bae5a 100644 --- a/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb +++ b/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb @@ -48,11 +48,6 @@ module Client # rubocop:disable Metrics/ModuleLength FULL_SQL_REGEXP = Regexp.union(MYSQL_COMPONENTS.map { |component| COMPONENTS_REGEX_MAP[component] }) - def initialize(args) - @_otel_net_peer_name = args[:host] - super - end - def query(sql) tracer.in_span( database_span_name(sql), @@ -68,11 +63,12 @@ def query(sql) def client_attributes(sql) attributes = { ::OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM => 'mysql', - ::OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => net_peer_name + ::OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => connection_options.fetch(:host, 'unknown sock'), } attributes[::OpenTelemetry::SemanticConventions::Trace::DB_NAME] = database_name if database_name attributes[::OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = config[:peer_service] unless config[:peer_service].nil? + attributes['db.mysql.instance.host.name'] = @connected_host if defined?(@connected_host) case config[:db_statement] when :obfuscate @@ -131,16 +127,6 @@ def database_name connection_options[:database] end - def net_peer_name - if defined?(@connected_host) - @connected_host - elsif @_otel_net_peer_name - @_otel_net_peer_name - else - 'unknown sock' - end - end - def tracer Trilogy::Instrumentation.instance.tracer end diff --git a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb index 2be1304c0a..a5f396c3f6 100644 --- a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb +++ b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb @@ -128,13 +128,10 @@ client.query('SELECT 1') _(span.name).must_equal 'select' - _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_NAME]).must_equal(database) - _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'SELECT ?' - _(span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) end - it 'includes database name' do + it 'includes database connection information' do client.query('SELECT 1') _(span.name).must_equal 'select' @@ -142,6 +139,7 @@ _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'SELECT ?' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) + _(span.attributes['db.mysql.instance.host.name']).must_be_nil end it 'extracts statement type' do @@ -175,6 +173,7 @@ _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'select @@hostname' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) + _(span.attributes['db.mysql.instance.host.name']).must_be_nil client.query('SELECT 1') @@ -184,8 +183,8 @@ _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_NAME]).must_equal(database) _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'SELECT ?' - _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).wont_equal(host) - _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal client.connected_host + _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) + _(last_span.attributes['db.mysql.instance.host.name']).must_equal client.connected_host end end From ad77dd54b4583bd18058d138edb210143a068b30 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Wed, 31 May 2023 17:15:19 -0500 Subject: [PATCH 2/5] Update instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb Co-authored-by: Christopher Schleiden --- .../lib/opentelemetry/instrumentation/trilogy/patches/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb b/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb index 71300bae5a..377bd48394 100644 --- a/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb +++ b/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb @@ -63,7 +63,7 @@ def query(sql) def client_attributes(sql) attributes = { ::OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM => 'mysql', - ::OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => connection_options.fetch(:host, 'unknown sock'), + ::OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => connection_options.fetch(:host, 'unknown sock') } attributes[::OpenTelemetry::SemanticConventions::Trace::DB_NAME] = database_name if database_name From baef1f08a5b53e8184e3333ecf40c6d8bba511fd Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Thu, 1 Jun 2023 00:35:43 +0000 Subject: [PATCH 3/5] squash: use address suffix --- .../instrumentation/trilogy/patches/client.rb | 2 +- .../trilogy/instrumentation_test.rb | 25 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb b/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb index 377bd48394..7c29318100 100644 --- a/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb +++ b/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb @@ -68,7 +68,7 @@ def client_attributes(sql) attributes[::OpenTelemetry::SemanticConventions::Trace::DB_NAME] = database_name if database_name attributes[::OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = config[:peer_service] unless config[:peer_service].nil? - attributes['db.mysql.instance.host.name'] = @connected_host if defined?(@connected_host) + attributes['db.mysql.instance.address'] = @connected_host if defined?(@connected_host) case config[:db_statement] when :obfuscate diff --git a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb index a5f396c3f6..7975754d88 100644 --- a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb +++ b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb @@ -73,25 +73,24 @@ describe '#compatible?' do describe 'when an unsupported version is installed' do it 'is incompatible' do - stub_const('Trilogy::VERSION', '2.0.0.beta') do - _(instrumentation.compatible?).must_equal false - end + stub_const('Trilogy::VERSION', '2.2.0') + _(instrumentation.compatible?).must_equal false - stub_const('Trilogy::VERSION', '3.0.0') do - _(instrumentation.compatible?).must_equal false - end + stub_const('Trilogy::VERSION', '2.3.0.beta') + _(instrumentation.compatible?).must_equal false + + stub_const('Trilogy::VERSION', '3.0.0') + _(instrumentation.compatible?).must_equal false end end describe 'when supported version is installed' do it 'is compatible' do - stub_const('Trilogy::VERSION', '2.0.0') do - _(instrumentation.compatible?).must_equal true - end + stub_const('Trilogy::VERSION', '2.3.0') + _(instrumentation.compatible?).must_equal true - stub_const('Trilogy::VERSION', '3.0.0.rc1') do - _(instrumentation.compatible?).must_equal true - end + stub_const('Trilogy::VERSION', '3.0.0.rc1') + _(instrumentation.compatible?).must_equal true end end end @@ -184,7 +183,7 @@ _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'SELECT ?' _(last_span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) - _(last_span.attributes['db.mysql.instance.host.name']).must_equal client.connected_host + _(last_span.attributes['db.mysql.instance.address']).must_equal client.connected_host end end From e1ede30723c53f142c8df25c5a85fe55eb0734ca Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Thu, 1 Jun 2023 14:11:26 +0000 Subject: [PATCH 4/5] squash: update tests --- .../instrumentation/trilogy/instrumentation_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb index 7975754d88..dd8131b9d8 100644 --- a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb +++ b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb @@ -138,7 +138,7 @@ _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'SELECT ?' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) - _(span.attributes['db.mysql.instance.host.name']).must_be_nil + _(span.attributes[''db.mysql.instance.address']).must_be_nil end it 'extracts statement type' do @@ -172,7 +172,7 @@ _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'select @@hostname' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) - _(span.attributes['db.mysql.instance.host.name']).must_be_nil + _(span.attributes[''db.mysql.instance.address']).must_be_nil client.query('SELECT 1') From ff5a336a0611aaea9a6da4c75714daaa067f20e6 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Thu, 1 Jun 2023 14:18:23 +0000 Subject: [PATCH 5/5] squash: fix syntax error --- .../instrumentation/trilogy/instrumentation_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb index dd8131b9d8..fb6ae45312 100644 --- a/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb +++ b/instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb @@ -138,7 +138,7 @@ _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'SELECT ?' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) - _(span.attributes[''db.mysql.instance.address']).must_be_nil + _(span.attributes['db.mysql.instance.address']).must_be_nil end it 'extracts statement type' do @@ -172,7 +172,7 @@ _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_SYSTEM]).must_equal 'mysql' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::DB_STATEMENT]).must_equal 'select @@hostname' _(span.attributes[OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME]).must_equal(host) - _(span.attributes[''db.mysql.instance.address']).must_be_nil + _(span.attributes['db.mysql.instance.address']).must_be_nil client.query('SELECT 1')