From 989da17c33c345ad285f70a0821078a6e21d389d Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Thu, 20 Jun 2024 15:09:19 -0500 Subject: [PATCH] perf: Reduce Context Allocations in ActiveJob (#1018) * perf: Reduce Context Allocations in ActiveJob After seeing changes to Rack::Events that reduced the number of active contexts required for ingress spans, I decided to apply the same changes to ActiveJob * doc: Update default.rb Co-authored-by: Steven Harman --------- Co-authored-by: Steven Harman --- instrumentation/active_job/Appraisals | 4 ++++ .../active_job/handlers/default.rb | 15 +++++++-------- .../active_job/handlers/enqueue.rb | 3 +-- .../active_job/handlers/perform.rb | 10 +++------- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/instrumentation/active_job/Appraisals b/instrumentation/active_job/Appraisals index 05429cd62..a25edd334 100644 --- a/instrumentation/active_job/Appraisals +++ b/instrumentation/active_job/Appraisals @@ -9,3 +9,7 @@ gem 'activejob', "~> #{version}" end end + +appraise 'activejob-latest' do + gem 'activejob' +end diff --git a/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/default.rb b/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/default.rb index 9a5db43b5..692346aeb 100644 --- a/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/default.rb +++ b/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/default.rb @@ -41,9 +41,9 @@ def start(name, id, payload) # @return [Hash] with the span and generated context tokens def start_span(name, _id, payload) span = tracer.start_span(name, attributes: @mapper.call(payload)) - tokens = [OpenTelemetry::Context.attach(OpenTelemetry::Trace.context_with_span(span))] + token = OpenTelemetry::Context.attach(OpenTelemetry::Trace.context_with_span(span)) - { span: span, ctx_tokens: tokens } + { span: span, ctx_token: token } end # Creates a span and registers it with the current context @@ -55,20 +55,20 @@ def start_span(name, _id, payload) def finish(_name, _id, payload) otel = payload.delete(:__otel) span = otel&.fetch(:span) - tokens = otel&.fetch(:ctx_tokens) + token = otel&.fetch(:ctx_token) on_exception((payload[:error] || payload[:exception_object]), span) rescue StandardError => e OpenTelemetry.handle_error(exception: e) ensure - finish_span(span, tokens) + finish_span(span, token) end # Finishes the provided spans and also detaches the associated contexts # # @param span [OpenTelemetry::Trace::Span] - # @param tokens [Array] to unregister - def finish_span(span, tokens) + # @param token [Numeric] to unregister + def finish_span(span, token) # closes the span after all attributes have been finalized begin if span&.recording? @@ -79,8 +79,7 @@ def finish_span(span, tokens) OpenTelemetry.handle_error(exception: e) end - # pops the context stack - tokens&.reverse_each do |token| + begin OpenTelemetry::Context.detach(token) rescue StandardError => e OpenTelemetry.handle_error(exception: e) diff --git a/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/enqueue.rb b/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/enqueue.rb index 7dace8a95..4b4c55937 100644 --- a/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/enqueue.rb +++ b/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/enqueue.rb @@ -29,9 +29,8 @@ def initialize(...) def start_span(name, _id, payload) job = payload.fetch(:job) span = tracer.start_span(@span_name_formatter.call(job), kind: :producer, attributes: @mapper.call(payload)) - tokens = [OpenTelemetry::Context.attach(OpenTelemetry::Trace.context_with_span(span))] OpenTelemetry.propagation.inject(job.__otel_headers) # This must be transmitted over the wire - { span: span, ctx_tokens: tokens } + { span: span, ctx_token: OpenTelemetry::Context.attach(OpenTelemetry::Trace.context_with_span(span)) } end end end diff --git a/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/perform.rb b/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/perform.rb index 6dd7e2e28..bbfd2ed27 100644 --- a/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/perform.rb +++ b/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/handlers/perform.rb @@ -27,7 +27,6 @@ def initialize(...) # @param payload [Hash] containing job run information # @return [Hash] with the span and generated context tokens def start_span(name, _id, payload) - tokens = [] job = payload.fetch(:job) parent_context = OpenTelemetry.propagation.extract(job.__otel_headers) @@ -36,7 +35,6 @@ def start_span(name, _id, payload) # TODO: Refactor into a propagation strategy propagation_style = @config[:propagation_style] if propagation_style == :child - tokens << OpenTelemetry::Context.attach(parent_context) span = tracer.start_span(span_name, kind: :consumer, attributes: @mapper.call(payload)) else span_context = OpenTelemetry::Trace.current_span(parent_context).context @@ -44,9 +42,7 @@ def start_span(name, _id, payload) span = tracer.start_root_span(span_name, kind: :consumer, attributes: @mapper.call(payload), links: links) end - tokens.concat(attach_consumer_context(span)) - - { span: span, ctx_tokens: tokens } + { span: span, ctx_token: attach_consumer_context(span) } end # This method attaches a span to multiple contexts: @@ -54,12 +50,12 @@ def start_span(name, _id, payload) # This is used later to enrich the ingress span in children, e.g. setting span status to error when a child event like `discard` terminates due to an error # 2. Registers the ingress span as the "active" span, which is the default behavior of the SDK. # @param span [OpenTelemetry::Trace::Span] the currently active span used to record the exception and set the status - # @return [Array] Context tokens that must be detached when finished + # @return [Numeric] Context token that must be detached when finished def attach_consumer_context(span) consumer_context = OpenTelemetry::Trace.context_with_span(span) internal_context = OpenTelemetry::Instrumentation::ActiveJob.context_with_span(span, parent_context: consumer_context) - [consumer_context, internal_context].map { |context| OpenTelemetry::Context.attach(context) } + OpenTelemetry::Context.attach(internal_context) end end end