From 4fe2450d549d78ac3fa06227b8c3f096bb6d5e52 Mon Sep 17 00:00:00 2001 From: Nick Amorim Date: Mon, 14 Apr 2025 13:57:17 -0400 Subject: [PATCH 1/2] feat: support meta protocol instrumentation --- .../lib/opentelemetry/instrumentation/dalli/instrumentation.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/instrumentation.rb b/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/instrumentation.rb index 610bf5a5fc..190933eade 100644 --- a/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/instrumentation.rb +++ b/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/instrumentation.rb @@ -34,6 +34,7 @@ def add_patches ::Dalli::Server.prepend(Patches::Server) else ::Dalli::Protocol::Binary.prepend(Patches::Server) + ::Dalli::Protocol::Meta.prepend(Patches::Server) if defined?(::Dalli::Protocol::Meta) end end end From b12e3f8e2ef4e1b2b94f7c9f1731ebfefcb97cac Mon Sep 17 00:00:00 2001 From: Nick Amorim Date: Mon, 14 Apr 2025 17:09:35 -0400 Subject: [PATCH 2/2] test: dalli meta and binary protocol instrumentation support --- .../instrumentation/dalli/patches/server.rb | 2 +- .../dalli/instrumentation_test.rb | 182 +++++++++--------- 2 files changed, 95 insertions(+), 89 deletions(-) diff --git a/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/patches/server.rb b/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/patches/server.rb index e9fc96a962..3d1c102740 100644 --- a/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/patches/server.rb +++ b/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/patches/server.rb @@ -8,7 +8,7 @@ module OpenTelemetry module Instrumentation module Dalli module Patches - # Module to prepend to Dalli::Server (or Dalli::Protocol::Binary in 3.0+) for instrumentation + # Module to prepend to Dalli::Server (or Dalli::Protocol::Binary/Meta in 3.0+) for instrumentation module Server def request(op, *args) # rubocop:disable Naming/MethodParameterName operation = Utils.opname(op, multi?) diff --git a/instrumentation/dalli/test/opentelemetry/instrumentation/dalli/instrumentation_test.rb b/instrumentation/dalli/test/opentelemetry/instrumentation/dalli/instrumentation_test.rb index 37d3e4bf0a..434c185397 100644 --- a/instrumentation/dalli/test/opentelemetry/instrumentation/dalli/instrumentation_test.rb +++ b/instrumentation/dalli/test/opentelemetry/instrumentation/dalli/instrumentation_test.rb @@ -15,7 +15,6 @@ let(:span) { exporter.finished_spans.first } let(:host) { ENV.fetch('TEST_MEMCACHED_HOST', '127.0.0.1') } let(:port) { ENV.fetch('TEST_MEMCACHED_PORT', 11_211).to_i } - let(:dalli) { Dalli::Client.new("#{host}:#{port}", {}) } before do exporter.reset @@ -26,116 +25,123 @@ instrumentation.instance_variable_set(:@installed, false) end - describe 'tracing' do - before do - instrumentation.install(db_statement: :include) - end - - it 'accepts peer service name from config' do - instrumentation.instance_variable_set(:@installed, false) - instrumentation.install(peer_service: 'readonly:memcached') - dalli.set('foo', 'bar') - - _(span.attributes['peer.service']).must_equal 'readonly:memcached' - end + [ + ['binary protocol', { protocol: :binary }], + ['meta protocol', { protocol: :meta }] + ].each do |name, protocol| + describe "tracing with #{name}" do + let(:dalli) { Dalli::Client.new("#{host}:#{port}", protocol) } - it 'before request' do - _(exporter.finished_spans.size).must_equal 0 - end + before do + instrumentation.install(db_statement: :include) + end - it 'after dalli#set' do - dalli.set('foo', 'bar') + it 'accepts peer service name from config' do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(peer_service: 'readonly:memcached') + dalli.set('foo', 'bar') - _(exporter.finished_spans.size).must_equal 1 - _(span.name).must_equal 'set' - _(span.attributes['db.system']).must_equal 'memcached' - _(span.attributes['db.statement']).must_equal 'set foo bar 0 0' - _(span.attributes['db.operation']).must_equal 'set' - _(span.attributes['net.peer.name']).must_equal host - _(span.attributes['net.peer.port']).must_equal port - end + _(span.attributes['peer.service']).must_equal 'readonly:memcached' + end - it 'after dalli#set' do - dalli.get('foo') + it 'before request' do + _(exporter.finished_spans.size).must_equal 0 + end - _(exporter.finished_spans.size).must_equal 1 - _(span.name).must_equal 'get' - _(span.attributes['db.system']).must_equal 'memcached' - _(span.attributes['db.statement']).must_equal 'get foo' - _(span.attributes['db.operation']).must_equal 'get' - _(span.attributes['net.peer.name']).must_equal host - _(span.attributes['net.peer.port']).must_equal port - end + it 'after dalli#set' do + dalli.set('foo', 'bar') - it 'after dalli#get_multi' do - dalli.get_multi('foo', 'bar') + _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal 'set' + _(span.attributes['db.system']).must_equal 'memcached' + _(span.attributes['db.statement']).must_equal 'set foo bar 0 0' + _(span.attributes['db.operation']).must_equal 'set' + _(span.attributes['net.peer.name']).must_equal host + _(span.attributes['net.peer.port']).must_equal port + end - _(exporter.finished_spans.size).must_equal 1 - _(span.name).must_equal 'getkq' - _(span.attributes['db.system']).must_equal 'memcached' - _(span.attributes['db.statement']).must_equal 'getkq foo bar' - _(span.attributes['db.operation']).must_equal 'getkq' - _(span.attributes['net.peer.name']).must_equal host - _(span.attributes['net.peer.port']).must_equal port - end + it 'after dalli#get' do + dalli.get('foo') - it 'after error' do - dalli.set('foo', 'bar') - exporter.reset + _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal 'get' + _(span.attributes['db.system']).must_equal 'memcached' + _(span.attributes['db.statement']).must_equal 'get foo' + _(span.attributes['db.operation']).must_equal 'get' + _(span.attributes['net.peer.name']).must_equal host + _(span.attributes['net.peer.port']).must_equal port + end - dalli.instance_variable_get(:@ring).servers.first.stub(:write, ->(_bytes) { raise Dalli::NetworkError }) do + it 'after dalli#get_multi' do dalli.get_multi('foo', 'bar') - end - if supports_retry_on_network_errors? - _(exporter.finished_spans.size).must_equal 2 - else _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal 'getkq' + _(span.attributes['db.system']).must_equal 'memcached' + _(span.attributes['db.statement']).must_equal 'getkq foo bar' + _(span.attributes['db.operation']).must_equal 'getkq' + _(span.attributes['net.peer.name']).must_equal host + _(span.attributes['net.peer.port']).must_equal port end - _(span.name).must_equal 'getkq' - _(span.attributes['db.system']).must_equal 'memcached' - _(span.attributes['db.statement']).must_equal 'getkq foo bar' - _(span.attributes['db.operation']).must_equal 'getkq' - _(span.attributes['net.peer.name']).must_equal host - _(span.attributes['net.peer.port']).must_equal port - - span_event = span.events.first - _(span_event.name).must_equal 'exception' - _(span_event.attributes['exception.type']).must_equal 'Dalli::NetworkError' - _(span_event.attributes['exception.message']).must_equal 'Dalli::NetworkError' - end + it 'after error' do + dalli.set('foo', 'bar') + exporter.reset + + dalli.instance_variable_get(:@ring).servers.first.stub(:write, ->(_bytes) { raise Dalli::NetworkError }) do + dalli.get_multi('foo', 'bar') + end + + if supports_retry_on_network_errors? + _(exporter.finished_spans.size).must_equal 2 + else + _(exporter.finished_spans.size).must_equal 1 + end + + _(span.name).must_equal 'getkq' + _(span.attributes['db.system']).must_equal 'memcached' + _(span.attributes['db.statement']).must_equal 'getkq foo bar' + _(span.attributes['db.operation']).must_equal 'getkq' + _(span.attributes['net.peer.name']).must_equal host + _(span.attributes['net.peer.port']).must_equal port + + span_event = span.events.first + _(span_event.name).must_equal 'exception' + _(span_event.attributes['exception.type']).must_equal 'Dalli::NetworkError' + _(span_event.attributes['exception.message']).must_equal 'Dalli::NetworkError' + end - it 'omits db.statement' do - instrumentation.instance_variable_set(:@installed, false) - instrumentation.install(db_statement: :omit) + it 'omits db.statement' do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :omit) - dalli.set('foo', 'bar') + dalli.set('foo', 'bar') - _(exporter.finished_spans.size).must_equal 1 - _(span.name).must_equal 'set' - _(span.attributes).wont_include 'db.statement' - end + _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal 'set' + _(span.attributes).wont_include 'db.statement' + end - it 'obfuscates db.statement' do - instrumentation.instance_variable_set(:@installed, false) - instrumentation.install(db_statement: :obfuscate) + it 'obfuscates db.statement' do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :obfuscate) - dalli.set('foo', 'bar') + dalli.set('foo', 'bar') - _(exporter.finished_spans.size).must_equal 1 - _(span.name).must_equal 'set' - _(span.attributes['db.statement']).must_equal 'set ?' - end + _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal 'set' + _(span.attributes['db.statement']).must_equal 'set ?' + end - it 'supports gat' do - skip unless dalli.respond_to?(:gat) + it 'supports gat' do + skip unless dalli.respond_to?(:gat) - dalli.gat('foo') + dalli.gat('foo') - _(exporter.finished_spans.size).must_equal 1 - _(span.name).must_equal 'gat' - _(span.attributes['db.statement']).must_equal 'gat foo 0' + _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal 'gat' + _(span.attributes['db.statement']).must_equal 'gat foo 0' + end end end