Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: OTEL_TRACES_EXPORTER and OTEL_PROPAGATORS #604

Merged
merged 6 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ gemspec
# Use the opentelemetry-api gem from source
gem 'opentelemetry-api', path: '../api'
gem 'opentelemetry-common', path: '../common'
gem 'opentelemetry-exporter-jaeger', path: '../exporter/jaeger'
63 changes: 44 additions & 19 deletions sdk/lib/opentelemetry/sdk/configurator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,41 +143,66 @@ def install_instrumentation
end

def configure_span_processors
processors = @span_processors.empty? ? [default_span_processor] : @span_processors
processors = @span_processors.empty? ? [wrapped_exporter_from_env].compact : @span_processors
processors.each { |p| tracer_provider.add_span_processor(p) }
end

def default_span_processor
Trace::Export::SimpleSpanProcessor.new(
Trace::Export::ConsoleSpanExporter.new
)
def wrapped_exporter_from_env
exporter = ENV.fetch('OTEL_TRACES_EXPORTER', 'otlp')
case exporter
when 'none' then nil
when 'otlp' then fetch_exporter(exporter, 'OpenTelemetry::Exporter::OTLP::Exporter')
when 'jaeger' then fetch_exporter(exporter, 'OpenTelemetry::Exporter::Jaeger::CollectorExporter')
when 'zipkin' then fetch_exporter(exporter, 'OpenTelemetry::Exporter::Zipkin::Exporter')
else
OpenTelemetry.logger.warn "The #{exporter} exporter is unknown and cannot be configured, spans will not be exported"
nil
Comment on lines +146 to +159
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ([wrapped_exporter_from_env].compact and returning nil in the default cases) means we end up with the default NoopSpanProcessor rather than adding another one.

end
end

def configure_propagation
OpenTelemetry.propagation = create_propagator(@injectors || default_injectors,
@extractors || default_extractors)
def configure_propagation # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
propagators = ENV.fetch('OTEL_PROPAGATORS', 'tracecontext,baggage').split(',')
injectors, extractors = propagators.uniq.collect do |propagator|
case propagator
when 'tracecontext'
[OpenTelemetry::Trace::Propagation::TraceContext.text_map_injector, OpenTelemetry::Trace::Propagation::TraceContext.text_map_extractor]
when 'baggage'
[OpenTelemetry::Baggage::Propagation.text_map_injector, OpenTelemetry::Baggage::Propagation.text_map_extractor]
when 'b3' then fetch_propagator(propagator, 'OpenTelemetry::Propagator::B3::Single')
when 'b3multi' then fetch_propagator(propagator, 'OpenTelemetry::Propagator::B3::Multi', 'b3')
when 'jaeger' then fetch_propagator(propagator, 'OpenTelemetry::Propagator::Jaeger')
when 'xray' then fetch_propagator(propagator, 'OpenTelemetry::Propagator::XRay')
ericmustin marked this conversation as resolved.
Show resolved Hide resolved
when 'ottrace' then fetch_propagator(propagator, 'OpenTelemetry::Propagator::OTTrace')
else
OpenTelemetry.logger.warn "The #{propagator} propagator is unknown and cannot be configured"
[Context::Propagation::NoopInjector.new, Context::Propagation::NoopExtractor.new]
end
end.transpose
OpenTelemetry.propagation = create_propagator(@injectors || injectors.compact,
@extractors || extractors.compact)
end

def create_propagator(injectors, extractors)
if injectors.size > 1 || extractors.size > 1
Context::Propagation::CompositePropagator.new(injectors, extractors)
else
Context::Propagation::Propagator.new(injectors, extractors)
Context::Propagation::Propagator.new(injectors.first, extractors.first)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug exposed by new testing here.

end
end

def default_injectors
[
OpenTelemetry::Trace::Propagation::TraceContext.text_map_injector,
OpenTelemetry::Baggage::Propagation.text_map_injector
]
def fetch_propagator(name, class_name, gem_suffix = name)
propagator_class = Kernel.const_get(class_name)
[propagator_class.text_map_injector, propagator_class.text_map_extractor]
rescue NameError
OpenTelemetry.logger.warn "The #{name} propagator cannot be configured - please add opentelemetry-propagator-#{gem_suffix} to your Gemfile"
[nil, nil]
end

def default_extractors
[
OpenTelemetry::Trace::Propagation::TraceContext.text_map_extractor,
OpenTelemetry::Baggage::Propagation.text_map_extractor
]
def fetch_exporter(name, class_name)
Trace::Export::BatchSpanProcessor.new(Kernel.const_get(class_name).new)
rescue NameError
OpenTelemetry.logger.warn "The #{name} exporter cannot be configured - please add opentelemetry-exporter-#{name} to your Gemfile, spans will not be exported"
nil
end
end
end
Expand Down
1 change: 1 addition & 0 deletions sdk/opentelemetry-sdk.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'bundler', '>= 1.17'
spec.add_development_dependency 'faraday', '~> 0.13'
spec.add_development_dependency 'minitest', '~> 5.0'
spec.add_development_dependency 'opentelemetry-exporter-jaeger', '~> 0.14.0'
spec.add_development_dependency 'rake', '~> 12.0'
spec.add_development_dependency 'rubocop', '~> 0.73.0'
spec.add_development_dependency 'simplecov', '~> 0.17'
Expand Down
82 changes: 78 additions & 4 deletions sdk/test/opentelemetry/sdk/configurator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# SPDX-License-Identifier: Apache-2.0

require 'test_helper'
require 'opentelemetry/exporter/jaeger'

describe OpenTelemetry::SDK::Configurator do
let(:configurator) { OpenTelemetry::SDK::Configurator.new }
Expand Down Expand Up @@ -134,6 +135,27 @@

_(injectors_for(OpenTelemetry.propagation)).must_equal([injector])
end

it 'can be set by environment variable' do
expected_injectors = [OpenTelemetry::Baggage::Propagation.text_map_injector]

with_env('OTEL_PROPAGATORS' => 'baggage') do
configurator.configure
end

_(injectors_for(OpenTelemetry.propagation)).must_equal(expected_injectors)
end

it 'defaults to none with invalid env var' do
with_env('OTEL_PROPAGATORS' => 'unladen_swallow') do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

configurator.configure
end

_(injectors_for(OpenTelemetry.propagation).length).must_equal(1)
_(injectors_for(OpenTelemetry.propagation).first).must_be_instance_of(
Context::Propagation::NoopInjector
)
end
end

describe '#extractors' do
Expand All @@ -155,6 +177,27 @@

_(extractors_for(OpenTelemetry.propagation)).must_equal([extractor])
end

it 'can be set by environment variable' do
expected_extractors = [OpenTelemetry::Baggage::Propagation.text_map_extractor]

with_env('OTEL_PROPAGATORS' => 'baggage') do
configurator.configure
end

_(extractors_for(OpenTelemetry.propagation)).must_equal(expected_extractors)
end

it 'defaults to none with invalid env var' do
with_env('OTEL_PROPAGATORS' => 'unladen_swallow') do
configurator.configure
end

_(extractors_for(OpenTelemetry.propagation).length).must_equal(1)
_(extractors_for(OpenTelemetry.propagation).first).must_be_instance_of(
Context::Propagation::NoopExtractor
)
end
end

describe 'tracer_provider' do
Expand All @@ -176,11 +219,11 @@
end

describe 'span processors' do
it 'defaults to SimpleSpanProcessor w/ ConsoleSpanExporter' do
it 'defaults to NoopSpanProcessor if no valid exporter is available' do
configurator.configure

_(OpenTelemetry.tracer_provider.active_span_processor).must_be_instance_of(
OpenTelemetry::SDK::Trace::Export::SimpleSpanProcessor
OpenTelemetry::SDK::Trace::NoopSpanProcessor
)
end

Expand All @@ -197,6 +240,29 @@
OpenTelemetry::SDK::Trace::Export::BatchSpanProcessor
)
end

it 'can be set by environment variable' do
with_env('OTEL_TRACES_EXPORTER' => 'jaeger') do
configurator.configure
end

_(OpenTelemetry.tracer_provider.active_span_processor).must_be_instance_of(
OpenTelemetry::SDK::Trace::Export::BatchSpanProcessor
)
_(OpenTelemetry.tracer_provider.active_span_processor.instance_variable_get(:@exporter)).must_be_instance_of(
OpenTelemetry::Exporter::Jaeger::CollectorExporter
)
end

it 'accepts "none" as an environment variable value' do
with_env('OTEL_TRACES_EXPORTER' => 'none') do
configurator.configure
end

_(OpenTelemetry.tracer_provider.active_span_processor).must_be_instance_of(
OpenTelemetry::SDK::Trace::NoopSpanProcessor
)
end
end

describe 'instrumentation installation' do
Expand Down Expand Up @@ -238,10 +304,18 @@
end

def extractors_for(propagator)
propagator.instance_variable_get(:@extractors) || propagator.instance_variable_get(:@extractor)
if propagator.instance_of? Context::Propagation::CompositePropagator
propagator.instance_variable_get(:@extractors)
else
[propagator.instance_variable_get(:@extractor)]
end
Comment on lines +307 to +311
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes some noise in the CI logs.

end

def injectors_for(propagator)
propagator.instance_variable_get(:@injectors) || propagator.instance_variable_get(:@injector)
if propagator.instance_of? Context::Propagation::CompositePropagator
propagator.instance_variable_get(:@injectors)
else
[propagator.instance_variable_get(:@injector)]
end
end
end