Merge branch 'an-dtracing-test-for-invalid-tracers' into 'master'

Avoid overwriting default jaeger values with nil

See merge request gitlab-org/gitlab-ce!24482

3 files changed, 48 insertions, 17 deletions

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 00000000000..5365260cbae
--- /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 0726f6b67f4..2682007302a 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 3bffeb28830..3d6a007cfd9 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