From c7466ae63cfeac642d839237032345900280b6d5 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 9 Dec 2021 09:54:50 +0100 Subject: [PATCH 1/4] Use stack for mongo spy spans --- lib/elastic_apm/spies/mongo.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/elastic_apm/spies/mongo.rb b/lib/elastic_apm/spies/mongo.rb index 8e04f81e8..6ea60c2d7 100644 --- a/lib/elastic_apm/spies/mongo.rb +++ b/lib/elastic_apm/spies/mongo.rb @@ -38,7 +38,7 @@ class Subscriber EVENT_KEY = :__elastic_instrumenter_mongo_events_key def events - Thread.current[EVENT_KEY] ||= {} + Thread.current[EVENT_KEY] ||= [] end def started(event) @@ -91,14 +91,13 @@ def push_event(event) context: build_context(event) ) - events[event.operation_id] = span + events << span end def pop_event(event) - span = events.delete(event.operation_id) return unless (curr = ElasticAPM.current_span) - curr == span && ElasticAPM.end_span + curr == events[-1] && ElasticAPM.end_span(events.pop) end def build_context(event) From df6117c682236480658680a4973efeb103a569d8 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 9 Dec 2021 11:39:55 +0100 Subject: [PATCH 2/4] Add mutex so that the correct current transaction and span are consulted --- lib/elastic_apm/spies/mongo.rb | 68 +++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/lib/elastic_apm/spies/mongo.rb b/lib/elastic_apm/spies/mongo.rb index 6ea60c2d7..dd26a5998 100644 --- a/lib/elastic_apm/spies/mongo.rb +++ b/lib/elastic_apm/spies/mongo.rb @@ -41,6 +41,10 @@ def events Thread.current[EVENT_KEY] ||= [] end + def initialize + @mutex = Mutex.new + end + def started(event) push_event(event) end @@ -64,40 +68,44 @@ def succeeded(event) private def push_event(event) - return unless ElasticAPM.current_transaction - # Some MongoDB commands are not on collections but rather are db - # admin commands. For these commands, the value at the `command_name` - # key is the integer 1. - # For getMore commands, the value at `command_name` is the cursor id - # and the collection name is at the key `collection` - collection = - if event.command[event.command_name] == 1 || - event.command[event.command_name].is_a?(BSON::Int64) - event.command[:collection] - else - event.command[event.command_name] - end - - name = [event.database_name, - collection, - event.command_name].compact.join('.') - - span = - ElasticAPM.start_span( - name, - TYPE, - subtype: SUBTYPE, - action: ACTION, - context: build_context(event) - ) - - events << span + @mutex.synchronize do + return unless ElasticAPM.current_transaction + # Some MongoDB commands are not on collections but rather are db + # admin commands. For these commands, the value at the `command_name` + # key is the integer 1. + # For getMore commands, the value at `command_name` is the cursor id + # and the collection name is at the key `collection` + collection = + if event.command[event.command_name] == 1 || + event.command[event.command_name].is_a?(BSON::Int64) + event.command[:collection] + else + event.command[event.command_name] + end + + name = [event.database_name, + collection, + event.command_name].compact.join('.') + + span = + ElasticAPM.start_span( + name, + TYPE, + subtype: SUBTYPE, + action: ACTION, + context: build_context(event) + ) + + events << span + end end def pop_event(event) - return unless (curr = ElasticAPM.current_span) + @mutex.synchronize do + return unless (curr = ElasticAPM.current_span) - curr == events[-1] && ElasticAPM.end_span(events.pop) + curr == events[-1] && ElasticAPM.end_span(events.pop) + end end def build_context(event) From 92584ef863a832a4f42a73da9360145ac45ce9c6 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 9 Dec 2021 12:33:58 +0100 Subject: [PATCH 3/4] current span and transaction are thread locals so we don't need mutexes --- lib/elastic_apm/spies/mongo.rb | 48 ++++++++++++++++------------------ 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/lib/elastic_apm/spies/mongo.rb b/lib/elastic_apm/spies/mongo.rb index dd26a5998..9ab2f83f7 100644 --- a/lib/elastic_apm/spies/mongo.rb +++ b/lib/elastic_apm/spies/mongo.rb @@ -68,26 +68,25 @@ def succeeded(event) private def push_event(event) - @mutex.synchronize do - return unless ElasticAPM.current_transaction - # Some MongoDB commands are not on collections but rather are db - # admin commands. For these commands, the value at the `command_name` - # key is the integer 1. - # For getMore commands, the value at `command_name` is the cursor id - # and the collection name is at the key `collection` - collection = - if event.command[event.command_name] == 1 || - event.command[event.command_name].is_a?(BSON::Int64) - event.command[:collection] - else - event.command[event.command_name] - end - - name = [event.database_name, - collection, - event.command_name].compact.join('.') - - span = + return unless ElasticAPM.current_transaction + # Some MongoDB commands are not on collections but rather are db + # admin commands. For these commands, the value at the `command_name` + # key is the integer 1. + # For getMore commands, the value at `command_name` is the cursor id + # and the collection name is at the key `collection` + collection = + if event.command[event.command_name] == 1 || + event.command[event.command_name].is_a?(BSON::Int64) + event.command[:collection] + else + event.command[event.command_name] + end + + name = [event.database_name, + collection, + event.command_name].compact.join('.') + + span = ElasticAPM.start_span( name, TYPE, @@ -96,16 +95,13 @@ def push_event(event) context: build_context(event) ) - events << span - end + events << span end def pop_event(event) - @mutex.synchronize do - return unless (curr = ElasticAPM.current_span) + return unless (curr = ElasticAPM.current_span) - curr == events[-1] && ElasticAPM.end_span(events.pop) - end + curr == events[-1] && ElasticAPM.end_span(events.pop) end def build_context(event) From 38a5386b17a0d48ba66e26063169e244424b6611 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 9 Dec 2021 12:35:07 +0100 Subject: [PATCH 4/4] Fix spacing --- lib/elastic_apm/spies/mongo.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/lib/elastic_apm/spies/mongo.rb b/lib/elastic_apm/spies/mongo.rb index 9ab2f83f7..9b53e7008 100644 --- a/lib/elastic_apm/spies/mongo.rb +++ b/lib/elastic_apm/spies/mongo.rb @@ -41,10 +41,6 @@ def events Thread.current[EVENT_KEY] ||= [] end - def initialize - @mutex = Mutex.new - end - def started(event) push_event(event) end @@ -87,13 +83,13 @@ def push_event(event) event.command_name].compact.join('.') span = - ElasticAPM.start_span( - name, - TYPE, - subtype: SUBTYPE, - action: ACTION, - context: build_context(event) - ) + ElasticAPM.start_span( + name, + TYPE, + subtype: SUBTYPE, + action: ACTION, + context: build_context(event) + ) events << span end