diff --git a/changelogs/unreleased/an-dtracing-test-for-invalid-tracers.yml b/changelogs/unreleased/an-dtracing-test-for-invalid-tracers.yml new file mode 100644 index 0000000000000000000000000000000000000000..5365260cbaeddbe8bdf803c415b225bbc2824d41 --- /dev/null +++ b/changelogs/unreleased/an-dtracing-test-for-invalid-tracers.yml @@ -0,0 +1,5 @@ +--- +title: Avoid overwriting default jaeger values with nil +merge_request: 24482 +author: +type: fixed diff --git a/lib/gitlab/tracing/jaeger_factory.rb b/lib/gitlab/tracing/jaeger_factory.rb index 0726f6b67f4e2be7325d1dbf46b44c06241bb985..2682007302adb88099e50a50d6d054f5f960800a 100644 --- a/lib/gitlab/tracing/jaeger_factory.rb +++ b/lib/gitlab/tracing/jaeger_factory.rb @@ -22,7 +22,7 @@ module Gitlab service_name: service_name, sampler: get_sampler(options[:sampler], options[:sampler_param]), reporter: get_reporter(service_name, options[:http_endpoint], options[:udp_endpoint]) - } + }.compact extra_params = options.except(:sampler, :sampler_param, :http_endpoint, :udp_endpoint, :strict_parsing, :debug) # rubocop: disable CodeReuse/ActiveRecord if extra_params.present? diff --git a/spec/lib/gitlab/tracing/jaeger_factory_spec.rb b/spec/lib/gitlab/tracing/jaeger_factory_spec.rb index 3bffeb2883058ce7e7f96ea7fc5d777ac1614ed0..3d6a007cfd9e565ea42ed95ae604c6b9843afd38 100644 --- a/spec/lib/gitlab/tracing/jaeger_factory_spec.rb +++ b/spec/lib/gitlab/tracing/jaeger_factory_spec.rb @@ -6,36 +6,62 @@ describe Gitlab::Tracing::JaegerFactory do describe '.create_tracer' do let(:service_name) { 'rspec' } - it 'processes default connections' do - expect(described_class.create_tracer(service_name, {})).to respond_to(:active_span) + shared_examples_for 'a jaeger tracer' do + it 'responds to active_span methods' do + expect(tracer).to respond_to(:active_span) + end + + it 'yields control' do + expect { |b| tracer.start_active_span('operation_name', &b) }.to yield_control + end + end + + context 'processes default connections' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, {}) } + end end - it 'handles debug options' do - expect(described_class.create_tracer(service_name, { debug: "1" })).to respond_to(:active_span) + context 'handles debug options' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, { debug: "1" }) } + end end - it 'handles const sampler' do - expect(described_class.create_tracer(service_name, { sampler: "const", sampler_param: "1" })).to respond_to(:active_span) + context 'handles const sampler' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, { sampler: "const", sampler_param: "1" }) } + end end - it 'handles probabilistic sampler' do - expect(described_class.create_tracer(service_name, { sampler: "probabilistic", sampler_param: "0.5" })).to respond_to(:active_span) + context 'handles probabilistic sampler' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, { sampler: "probabilistic", sampler_param: "0.5" }) } + end end - it 'handles http_endpoint configurations' do - expect(described_class.create_tracer(service_name, { http_endpoint: "http://localhost:1234" })).to respond_to(:active_span) + context 'handles http_endpoint configurations' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, { http_endpoint: "http://localhost:1234" }) } + end end - it 'handles udp_endpoint configurations' do - expect(described_class.create_tracer(service_name, { udp_endpoint: "localhost:4321" })).to respond_to(:active_span) + context 'handles udp_endpoint configurations' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, { udp_endpoint: "localhost:4321" }) } + end end - it 'ignores invalid parameters' do - expect(described_class.create_tracer(service_name, { invalid: "true" })).to respond_to(:active_span) + context 'ignores invalid parameters' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, { invalid: "true" }) } + end end - it 'accepts the debug parameter when strict_parser is set' do - expect(described_class.create_tracer(service_name, { debug: "1", strict_parsing: "1" })).to respond_to(:active_span) + context 'accepts the debug parameter when strict_parser is set' do + it_behaves_like 'a jaeger tracer' do + let(:tracer) { described_class.create_tracer(service_name, { debug: "1", strict_parsing: "1" }) } + end end it 'rejects invalid parameters when strict_parser is set' do