From 1cd2d83f234fab1197ba0a710ae342db1d2fe890 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 22 Aug 2024 22:57:27 -0700 Subject: [PATCH] Move current_span to Tracer interface --- .../lib/aws-sdk-core/telemetry/base.rb | 14 ++++---- .../lib/aws-sdk-core/telemetry/no_op.rb | 6 ++-- .../lib/aws-sdk-core/telemetry/otel.rb | 15 ++++----- .../sig/aws-sdk-core/telemetry/base.rbs | 4 +-- .../spec/aws/telemetry/base_spec.rb | 8 ++--- .../spec/aws/telemetry/no_op_spec.rb | 7 ++++ .../spec/aws/telemetry/otel_spec.rb | 33 +++++++++---------- gems/aws-sdk-core/spec/aws/telemetry_spec.rb | 30 ----------------- 8 files changed, 46 insertions(+), 71 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/base.rb b/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/base.rb index 67078c4355e..0ae0b950ec5 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/base.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/base.rb @@ -65,6 +65,13 @@ def start_span(name, with_parent: nil, attributes: nil, kind: nil) def in_span(name, attributes: nil, kind: nil) raise NotImplementedError end + + # Returns the current active span. + # + # @return [Aws::Telemetry::SpanBase] + def current_span + raise NotImplementedError + end end # Base for `Span` classes. @@ -147,13 +154,6 @@ def current raise NotImplementedError end - # Returns the current span from current context. - # - # @return [Aws::Telemetry::SpanBase] - def current_span - raise NotImplementedError - end - # Associates a Context with the caller’s current execution unit. # Returns a token to be used with the matching call to detach. # diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/no_op.rb b/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/no_op.rb index e0ef1aaa9b4..5b7d855b249 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/no_op.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/no_op.rb @@ -28,6 +28,10 @@ def start_span(name, with_parent: nil, attributes: nil, kind: nil) def in_span(name, attributes: nil, kind: nil) yield NoOpSpan.new end + + def current_span + NoOpSpan.new + end end # No-op implementation for {SpanBase}. @@ -58,8 +62,6 @@ def record_exception(exception, attributes: nil); end class NoOpContextManager < ContextManagerBase def current; end - def current_span; end - def attach(context); end def detach(token); end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/otel.rb b/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/otel.rb index 7434c33d1ca..72ccc6f8c27 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/otel.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/telemetry/otel.rb @@ -45,7 +45,6 @@ def initialize private - # @api private def otel_loaded? if @use_otel.nil? @use_otel = @@ -117,6 +116,13 @@ def in_span(name, attributes: nil, kind: nil, &block) block.call(OTelSpan.new(span)) end end + + # Returns the current active span. + # + # @return [Aws::Telemetry::OTelSpan] + def current_span + OTelSpan.new(OpenTelemetry::Trace.current_span) + end end # OpenTelemetry-based {SpanBase}, represents a single operation @@ -206,13 +212,6 @@ def current OpenTelemetry::Context.current end - # Returns the current span from current context. - # - # @return [Aws::Telemetry::OTelSpan] - def current_span - OTelSpan.new(OpenTelemetry::Trace.current_span) - end - # Associates a Context with the caller’s current execution unit. # Returns a token to be used with the matching call to detach. # diff --git a/gems/aws-sdk-core/sig/aws-sdk-core/telemetry/base.rbs b/gems/aws-sdk-core/sig/aws-sdk-core/telemetry/base.rbs index ce9f9e4fb8f..3050bbc5710 100644 --- a/gems/aws-sdk-core/sig/aws-sdk-core/telemetry/base.rbs +++ b/gems/aws-sdk-core/sig/aws-sdk-core/telemetry/base.rbs @@ -15,6 +15,8 @@ module Aws def start_span: (String name, ?untyped with_parent, ?Hash[String, untyped] attributes, ?SpanKind kind) -> SpanBase def in_span: (String name, ?Hash[String, untyped] attributes, ?SpanKind kind) -> SpanBase + + def current_span: () -> SpanBase end class SpanBase @@ -35,8 +37,6 @@ module Aws class ContextManagerBase def current: () -> untyped - def current_span: () -> SpanBase - def attach: (untyped context) -> untyped def detach: (untyped token) -> bool diff --git a/gems/aws-sdk-core/spec/aws/telemetry/base_spec.rb b/gems/aws-sdk-core/spec/aws/telemetry/base_spec.rb index 1bf97633e7d..bb7ef0fc564 100644 --- a/gems/aws-sdk-core/spec/aws/telemetry/base_spec.rb +++ b/gems/aws-sdk-core/spec/aws/telemetry/base_spec.rb @@ -21,6 +21,10 @@ module Telemetry expect do subject.in_span('foo') end.to raise_error(NotImplementedError) + + expect do + subject.current_span + end.to raise_error(NotImplementedError) end end @@ -58,10 +62,6 @@ module Telemetry subject.current end.to raise_error(NotImplementedError) - expect do - subject.current_span - end.to raise_error(NotImplementedError) - expect do subject.attach('foo') end.to raise_error(NotImplementedError) diff --git a/gems/aws-sdk-core/spec/aws/telemetry/no_op_spec.rb b/gems/aws-sdk-core/spec/aws/telemetry/no_op_spec.rb index 382da5bb960..dec98bf5877 100644 --- a/gems/aws-sdk-core/spec/aws/telemetry/no_op_spec.rb +++ b/gems/aws-sdk-core/spec/aws/telemetry/no_op_spec.rb @@ -38,6 +38,13 @@ module Telemetry end end end + + describe '#current_span' do + it 'returns an instance of no-op span' do + expect(subject.current_span) + .to be_an_instance_of(Aws::Telemetry::NoOpSpan) + end + end end describe NoOpSpan do diff --git a/gems/aws-sdk-core/spec/aws/telemetry/otel_spec.rb b/gems/aws-sdk-core/spec/aws/telemetry/otel_spec.rb index 0f849ee6438..01502945322 100644 --- a/gems/aws-sdk-core/spec/aws/telemetry/otel_spec.rb +++ b/gems/aws-sdk-core/spec/aws/telemetry/otel_spec.rb @@ -6,21 +6,14 @@ module Aws module Telemetry describe OTelProvider do - before do - allow(Aws::Telemetry) - .to receive(:otel_loaded?) - .and_return(true) - end - let(:otel_provider) { OTelProvider.new } let(:context_manager) { otel_provider.context_manager } let(:tracer_provider) { otel_provider.tracer_provider } - let(:tracer) { tracer_provider.tracer('some_tracer') } describe '#initialize' do it 'raises ArgumentError when otel dependency fails to load' do allow_any_instance_of(Aws::Telemetry::OTelProvider) - .to receive(:otel_loaded?).and_return(false) + .to receive(:require).with('opentelemetry-sdk').and_raise(LoadError) expect { otel_provider }.to raise_error(ArgumentError) end @@ -43,14 +36,6 @@ module Telemetry end end - describe '#current_span' do - it 'returns the current span' do - wrapper_span = tracer.start_span('foo') - expect(context_manager.current_span.instance_variable_get(:@span)) - .to eq(wrapper_span.instance_variable_get(:@span)) - end - end - describe '#attach' do it 'sets the current context' do context_manager.attach(new_context) @@ -69,6 +54,8 @@ module Telemetry end describe 'OTelTracerProvider' do + let(:tracer) { tracer_provider.tracer('some_tracer') } + it 'returns a tracer instance' do expect(tracer).to be_a(Aws::Telemetry::OTelTracer) end @@ -76,6 +63,8 @@ module Telemetry context 'tracer' do let(:otel_export) { OpenTelemetry::SDK::Trace::Export } let(:otel_exporter) { otel_export::InMemorySpanExporter.new } + let(:finished_span) { otel_exporter.finished_spans[0] } + before do processor = otel_export::SimpleSpanProcessor.new(otel_exporter) OpenTelemetry::SDK.configure do |c| @@ -84,8 +73,6 @@ module Telemetry end after { SpecHelper.reset_opentelemetry_sdk } - let(:finished_span) { otel_exporter.finished_spans[0] } - describe '#start_span' do it 'returns a valid span with supplied parameters' do span = tracer.start_span('some_span') @@ -124,6 +111,16 @@ module Telemetry expect(finished_span.events[0].attributes['burnt']).to eq('pie') end end + + describe '#current_span' do + it 'returns the current span' do + tracer.in_span('foo') do |span| + span['blueberry'] = 'pie' + expect(tracer.current_span.instance_variable_get(:@span)) + .to eq(span.instance_variable_get(:@span)) + end + end + end end end end diff --git a/gems/aws-sdk-core/spec/aws/telemetry_spec.rb b/gems/aws-sdk-core/spec/aws/telemetry_spec.rb index 2ee1c397108..1b4236dfb01 100644 --- a/gems/aws-sdk-core/spec/aws/telemetry_spec.rb +++ b/gems/aws-sdk-core/spec/aws/telemetry_spec.rb @@ -5,36 +5,6 @@ module Aws describe Telemetry do - # describe '.otel_loaded?' do - # before { Aws::Telemetry.instance_variable_set(:@use_otel, nil) } - # - # it 'is true when opentelemetry-sdk is available' do - # expect(Aws::Telemetry).to receive(:require) - # .with('opentelemetry-sdk') - # .and_return(true) - # expect(Aws::Telemetry.otel_loaded?) - # .to be true - # end - # - # it 'returns false when opentelemetry-sdk is not available' do - # expect(Aws::Telemetry).to receive(:require) - # .with('opentelemetry-sdk') - # .and_raise(LoadError) - # expect(Aws::Telemetry.otel_loaded?) - # .to be false - # end - # - # it 'memoizes its status' do - # expect(Aws::Telemetry).to receive(:require) - # .once - # .with('opentelemetry-sdk') - # .and_raise(LoadError) - # Aws::Telemetry.otel_loaded? - # # second call should not call require again - # Aws::Telemetry.otel_loaded? - # end - # end - describe '.module_to_tracer_name' do it 'correctly converts module to tracer name' do expect(Aws::Telemetry.module_to_tracer_name('Aws::Telemetry'))