Merge pull request #89 from DataDog/anmarchenko/do_not_start_recorder…

…_when_ci_disabled

do not instantiate TestVisibility::Recorder unless CI visibility is enabled

lib/datadog/ci/configuration/components.rb

@@ -7,6 +7,7 @@
 require_relative "../ext/settings"
 require_relative "../test_visibility/flush"
 require_relative "../test_visibility/recorder"
+require_relative "../test_visibility/null_recorder"
 require_relative "../test_visibility/serializers/factories/test_level"
 require_relative "../test_visibility/serializers/factories/test_suite_level"
 require_relative "../test_visibility/transport"
@@ -21,14 +22,12 @@ module Components
 
         def initialize(settings)
           # Activate CI mode if enabled
-          activate_ci!(settings) if settings.ci.enabled
-
-          @ci_recorder = TestVisibility::Recorder.new(
-            enabled: settings.ci.enabled,
-            test_suite_level_visibility_enabled: settings.ci.experimental_test_suite_level_visibility_enabled
-          )
+          if settings.ci.enabled
+            activate_ci!(settings)
+          else
+            @ci_recorder = TestVisibility::NullRecorder.new
+          end
 
-          # Initialize normally
           super
         end
 
@@ -70,6 +69,10 @@ def activate_ci!(settings)
           end
 
           settings.tracing.test_mode.writer_options = writer_options
+
+          @ci_recorder = TestVisibility::Recorder.new(
+            test_suite_level_visibility_enabled: settings.ci.experimental_test_suite_level_visibility_enabled
+          )
         end
 
         def can_use_evp_proxy?(settings, agent_settings)

lib/datadog/ci/test_visibility/null_recorder.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+require_relative "recorder"
+
+module Datadog
+  module CI
+    module TestVisibility
+      # Special recorder that does not record anything
+      class NullRecorder
+        def start_test_session(service: nil, tags: {})
+          skip_tracing
+        end
+
+        def start_test_module(test_module_name, service: nil, tags: {})
+          skip_tracing
+        end
+
+        def start_test_suite(test_suite_name, service: nil, tags: {})
+          skip_tracing
+        end
+
+        def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block)
+          skip_tracing(block)
+        end
+
+        def trace(span_type, span_name, tags: {}, &block)
+          skip_tracing(block)
+        end
+
+        def active_span
+        end
+
+        def active_test
+        end
+
+        def active_test_session
+        end
+
+        def active_test_module
+        end
+
+        def active_test_suite(test_suite_name)
+        end
+
+        def deactivate_test(test)
+        end
+
+        def deactivate_test_session
+        end
+
+        def deactivate_test_module
+        end
+
+        def deactivate_test_suite(test_suite_name)
+        end
+
+        private
+
+        def skip_tracing(block = nil)
+          if block
+            block.call(null_span)
+          else
+            null_span
+          end
+        end
+
+        def null_span
+          @null_span ||= NullSpan.new
+        end
+      end
+    end
+  end
+end

lib/datadog/ci/test_visibility/recorder.rb

@@ -25,13 +25,12 @@ module TestVisibility
       # Common behavior for CI tests
       # Note: this class has too many responsibilities and should be split into multiple classes
       class Recorder
-        attr_reader :environment_tags, :test_suite_level_visibility_enabled, :enabled
+        attr_reader :environment_tags, :test_suite_level_visibility_enabled
 
-        def initialize(enabled: true, test_suite_level_visibility_enabled: false)
-          @enabled = enabled
-          @test_suite_level_visibility_enabled = enabled && test_suite_level_visibility_enabled
+        def initialize(test_suite_level_visibility_enabled: false)
+          @test_suite_level_visibility_enabled = test_suite_level_visibility_enabled
 
-          @environment_tags = @enabled ? Ext::Environment.tags(ENV).freeze : {}
+          @environment_tags = Ext::Environment.tags(ENV).freeze
           @local_context = Context::Local.new
           @global_context = Context::Global.new
         end
@@ -83,8 +82,6 @@ def start_test_suite(test_suite_name, service: nil, tags: {})
         end
 
         def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block)
-          return skip_tracing(block) unless enabled
-
           set_inherited_globals(tags)
           set_session_context(tags)
           set_module_context(tags)
@@ -118,8 +115,6 @@ def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block)
         end
 
         def trace(span_type, span_name, tags: {}, &block)
-          return skip_tracing(block) unless enabled
-
           span_options = build_span_options(
             nil, # service name is completely optional for custom spans
             span_type,

sig/datadog/ci/test_visibility/null_recorder.rbs

@@ -0,0 +1,45 @@
+module Datadog
+  module CI
+    module TestVisibility
+      class NullRecorder
+        @null_span: Datadog::CI::NullSpan
+
+        def initialize: (?untyped args) -> void
+
+        def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped
+
+        def trace: (String span_type, String span_name, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped
+
+        def start_test_session: (?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span
+
+        def start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span
+
+        def start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span
+
+        def active_test_session: () -> Datadog::CI::TestSession?
+
+        def active_test_module: () -> Datadog::CI::TestModule?
+
+        def active_test_suite: (String test_suite_name) -> Datadog::CI::TestSuite?
+
+        def active_test: () -> Datadog::CI::Test?
+
+        def active_span: () -> Datadog::CI::Span?
+
+        def deactivate_test: (Datadog::CI::Test test) -> void
+
+        def deactivate_test_session: () -> void
+
+        def deactivate_test_module: () -> void
+
+        def deactivate_test_suite: (String test_suite_name) -> void
+
+        private
+
+        def null_span: () -> Datadog::CI::Span
+
+        def skip_tracing: (?untyped block) -> untyped
+      end
+    end
+  end
+end

sig/datadog/ci/test_visibility/recorder.rbs

@@ -3,7 +3,6 @@ module Datadog
     module TestVisibility
       class Recorder
         @test_suite_level_visibility_enabled: bool
-        @enabled: bool
 
         @environment_tags: Hash[String, String]
         @local_context: Datadog::CI::TestVisibility::Context::Local
@@ -13,9 +12,8 @@ module Datadog
 
         attr_reader environment_tags: Hash[String, String]
         attr_reader test_suite_level_visibility_enabled: bool
-        attr_reader enabled: bool
 
-        def initialize: (?enabled: bool, ?test_suite_level_visibility_enabled: bool) -> void
+        def initialize: (?test_suite_level_visibility_enabled: bool) -> void
 
         def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped
 
@@ -45,12 +43,12 @@ module Datadog
 
         def deactivate_test_suite: (String test_suite_name) -> void
 
+        private
+
         def create_datadog_span: (String span_name, ?span_options: Hash[untyped, untyped], ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped
 
         def set_trace_origin: (Datadog::Tracing::TraceOperation trace) -> untyped
 
-        private
-
         def build_test: (Datadog::Tracing::SpanOperation tracer_span, Hash[untyped, untyped] tags) -> Datadog::CI::Test
 
         def build_test_session: (Datadog::Tracing::SpanOperation tracer_span, Hash[untyped, untyped] tags) -> Datadog::CI::TestSession

spec/datadog/ci/configuration/components_spec.rb

@@ -93,6 +93,9 @@
           allow(Datadog.logger)
             .to receive(:error)
 
+          allow(Datadog::CI::Ext::Environment)
+            .to receive(:tags).and_return({})
+
           components
         end
 
@@ -106,6 +109,10 @@
         context "is enabled" do
           let(:enabled) { true }
 
+          it "collects environment tags" do
+            expect(Datadog::CI::Ext::Environment).to have_received(:tags).with(ENV)
+          end
+
           context "when #experimental_test_suite_level_visibility_enabled" do
             context "is false" do
               it "creates a CI recorder with test_suite_level_visibility_enabled=false" do
@@ -237,6 +244,10 @@
             expect(settings.tracing.test_mode)
               .to_not have_received(:writer_options=)
           end
+
+          it "does not collect tags" do
+            expect(Datadog::CI::Ext::Environment).not_to have_received(:tags)
+          end
         end
       end
     end