Merge pull request #108 from DataDog/anmarchenko/trace_remove_type_param

[CIVIS-8333] improvements for manual tracing API

lib/datadog/ci.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 require_relative "ci/version"
+require_relative "ci/utils/configuration"
+require_relative "ci/ext/app_types"
 
 require "datadog/core"
 
@@ -9,6 +11,8 @@ module Datadog
   #
   # @public_api
   module CI
+    class ReservedTypeError < StandardError; end
+
     class << self
       # Starts a {Datadog::CI::TestSession ci_test_session} that represents the whole test session run.
       #
@@ -30,13 +34,11 @@ class << self
       #
       # Remember that calling {Datadog::CI::TestSession#finish} is mandatory.
       #
-      # @param [String] service the service name for this session (optional, defaults to DD_SERVICE)
+      # @param [String] service the service name for this session (optional, defaults to DD_SERVICE or repository name)
       # @param [Hash<String,String>] tags extra tags which should be added to the test session.
-      # @return [Datadog::CI::TestSession] returns the active, running {Datadog::CI::TestSession}.
-      # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled or if old Datadog agent is
-      #         detected and test suite level visibility cannot be supported.
-      def start_test_session(service: nil, tags: {})
-        service ||= Datadog.configuration.service
+      # @return [Datadog::CI::TestSession] the active, running {Datadog::CI::TestSession}.
+      # @return [nil] if test suite level visibility is disabled or CI mode is disabled.
+      def start_test_session(service: Utils::Configuration.fetch_service_name("test"), tags: {})
         recorder.start_test_session(service: service, tags: tags)
       end
 
@@ -88,9 +90,8 @@ def active_test_session
       # @param [String] test_module_name the name for this module
       # @param [String] service the service name for this session (optional, inherited from test session if not provided)
       # @param [Hash<String,String>] tags extra tags which should be added to the test module (optional, some tags are inherited from test session).
-      # @return [Datadog::CI::TestModule] returns the active, running {Datadog::CI::TestModule}.
-      # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled or if old Datadog agent is
-      #         detected and test suite level visibility cannot be supported.
+      # @return [Datadog::CI::TestModule] the active, running {Datadog::CI::TestModule}.
+      # @return [nil] if test suite level visibility is disabled or CI mode is disabled.
       def start_test_module(test_module_name, service: nil, tags: {})
         recorder.start_test_module(test_module_name, service: service, tags: tags)
       end
@@ -141,9 +142,8 @@ def active_test_module
       # @param [String] test_suite_name the name of the test suite
       # @param [String] service the service name for this test suite (optional, inherited from test session if not provided)
       # @param [Hash<String,String>] tags extra tags which should be added to the test module (optional, some tags are inherited from test session)
-      # @return [Datadog::CI::TestSuite] returns the active, running {Datadog::CI::TestSuite}.
-      # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled or if old Datadog agent is
-      #         detected and test suite level visibility cannot be supported.
+      # @return [Datadog::CI::TestSuite] the active, running {Datadog::CI::TestSuite}.
+      # @return [nil] if test suite level visibility is disabled or CI mode is disabled.
       def start_test_suite(test_suite_name, service: nil, tags: {})
         recorder.start_test_suite(test_suite_name, service: service, tags: tags)
       end
@@ -217,10 +217,10 @@ def active_test_suite(test_suite_name)
       # @return [Object] If a block is provided, returns the result of the block execution.
       # @return [Datadog::CI::Test] If no block is provided, returns the active,
       #         unfinished {Datadog::CI::Test}.
-      # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled
+      # @return [nil] if no block is provided and CI mode is disabled.
       # @yield Optional block where newly created {Datadog::CI::Test} captures the execution.
       # @yieldparam [Datadog::CI::Test] ci_test the newly created and active [Datadog::CI::Test]
-      # @yieldparam [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled
+      # @yieldparam [nil] if CI mode is disabled
       def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block)
         recorder.trace_test(test_name, test_suite_name, service: service, tags: tags, &block)
       end
@@ -245,8 +245,8 @@ def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block)
       # @param [String] test_suite_name name of test suite this test belongs to (example: "CalculatorTest").
       # @param [String] service the service name for this span (optional, inherited from test session if not provided)
       # @param [Hash<String,String>] tags extra tags which should be added to the test.
-      # @return [Datadog::CI::Test] Returns the active, unfinished {Datadog::CI::Test}.
-      # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled
+      # @return [Datadog::CI::Test] the active, unfinished {Datadog::CI::Test}.
+      # @return [nil] if CI mode is disabled.
       def start_test(test_name, test_suite_name, service: nil, tags: {})
         recorder.trace_test(test_name, test_suite_name, service: service, tags: tags)
       end
@@ -260,8 +260,8 @@ def start_test(test_name, test_suite_name, service: nil, tags: {})
       #
       # ```
       # Datadog::CI.trace(
-      #   "step",
       #   "Given I have 42 cucumbers",
+      #   type: "step",
       #   tags: {}
       # ) do
       #   run_operation
@@ -271,31 +271,40 @@ def start_test(test_name, test_suite_name, service: nil, tags: {})
       # The {.trace} method can also be used without a block in this way:
       # ```
       # ci_span = Datadog::CI.trace(
-      #   "step",
       #   "Given I have 42 cucumbers",
+      #   type: "step",
       #   tags: {}
       # )
       # # ... run test here ...
       # ci_span.finish
       # ```
       # Remember that in this case, calling {Datadog::CI::Span#finish} is mandatory.
       #
-      # @param [String] type custom, user-defined span type (for example "step" or "query").
       # @param [String] span_name the resource this span refers, or `test` if it's missing
+      # @param [String] type custom, user-defined span type (for example "step" or "query").
       # @param [Hash<String,String>] tags extra tags which should be added to the span.
       # @return [Object] If a block is provided, returns the result of the block execution.
       # @return [Datadog::CI::Span] If no block is provided, returns the active,
       #         unfinished {Datadog::CI::Span}.
-      # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled
+      # @return [nil] if CI visibility is disabled
+      # @raise [ReservedTypeError] if provided type is reserved for Datadog CI visibility
       # @yield Optional block where newly created {Datadog::CI::Span} captures the execution.
       # @yieldparam [Datadog::CI::Span] ci_span the newly created and active [Datadog::CI::Span]
-      # @yieldparam [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled
-      def trace(type, span_name, tags: {}, &block)
-        recorder.trace(type, span_name, tags: tags, &block)
+      # @yieldparam [nil] ci_span if CI visibility is disabled
+      def trace(span_name, type: "span", tags: {}, &block)
+        if Ext::AppTypes::CI_SPAN_TYPES.include?(type)
+          raise(
+            ReservedTypeError,
+            "Span type #{type} is reserved for Datadog CI visibility. " \
+              "Reserved types are: #{Ext::AppTypes::CI_SPAN_TYPES}"
+          )
+        end
+
+        recorder.trace(span_name, type: type, tags: tags, &block)
       end
 
-      # The active, unfinished custom span if it matches given type.
-      # If no span is active, or if the active span is not a custom span with given type, returns nil.
+      # The active, unfinished custom (i.e. not test/suite/module/session) span.
+      # If no span is active, or if the active span is not a custom span, returns nil.
       #
       # The active span belongs to an {.active_test}.
       #
@@ -314,12 +323,11 @@ def trace(type, span_name, tags: {}, &block)
       # step_span.finish()
       # ```
       #
-      # @param [String] type type of the span to retrieve (for example "step" or "query") that was provided to {.trace}
       # @return [Datadog::CI::Span] the active span
-      # @return [nil] if no span is active, or if the active span is not a custom span with given type
-      def active_span(type)
+      # @return [nil] if no span is active, or if the active span is not a custom span
+      def active_span
         span = recorder.active_span
-        span if span && span.type == type
+        span if span && !Ext::AppTypes::CI_SPAN_TYPES.include?(span.type)
       end
 
       # The active, unfinished test span.
@@ -347,30 +355,6 @@ def active_test
         recorder.active_test
       end
 
-      # Internal only, to finish a test use {Datadog::CI::Test#finish}
-      # @private
-      def deactivate_test(test)
-        recorder.deactivate_test(test)
-      end
-
-      # Internal only, to finish a test session use {Datadog::CI::TestSession#finish}
-      # @private
-      def deactivate_test_session
-        recorder.deactivate_test_session
-      end
-
-      # Internal only, to finish a test module use {Datadog::CI::TestModule#finish}
-      # @private
-      def deactivate_test_module
-        recorder.deactivate_test_module
-      end
-
-      # Internal only, to finish a test suite use {Datadog::CI::TestSuite#finish}
-      # @private
-      def deactivate_test_suite(test_suite_name)
-        recorder.deactivate_test_suite(test_suite_name)
-      end
-
       private
 
       def components

lib/datadog/ci/contrib/cucumber/formatter.rb

@@ -41,7 +41,7 @@ def on_test_run_started(event)
               },
               service: configuration[:service_name]
             )
-            CI.start_test_module(test_session.name)
+            CI.start_test_module(test_session.name) if test_session
           end
 
           def on_test_run_finished(event)
@@ -71,7 +71,7 @@ def on_test_case_started(event)
               service: configuration[:service_name]
             )
 
-            if (parameters = extract_parameters_hash(event.test_case))
+            if test_span && (parameters = extract_parameters_hash(event.test_case))
               test_span.set_parameters(parameters)
             end
           end
@@ -94,11 +94,11 @@ def on_test_case_finished(event)
           end
 
           def on_test_step_started(event)
-            CI.trace(Ext::STEP_SPAN_TYPE, event.test_step.to_s)
+            CI.trace(event.test_step.to_s, type: Ext::STEP_SPAN_TYPE)
           end
 
           def on_test_step_finished(event)
-            current_step_span = CI.active_span(Ext::STEP_SPAN_TYPE)
+            current_step_span = CI.active_span
             return if current_step_span.nil?
 
             finish_test(current_step_span, event.result)
@@ -157,7 +157,7 @@ def start_test_suite(test_suite_name)
 
             test_suite = CI.start_test_suite(test_suite_name)
             # will be overridden if any test fails
-            test_suite.passed!
+            test_suite.passed! if test_suite
 
             @current_test_suite = test_suite
           end

lib/datadog/ci/contrib/minitest/plugin.rb

@@ -54,7 +54,7 @@ def plugin_datadog_ci_init(*)
                 },
                 service: datadog_configuration[:service_name]
               )
-              CI.start_test_module(test_session.name)
+              CI.start_test_module(test_session.name) if test_session
 
               reporter.reporters << DatadogReporter.new(reporter)
             end

lib/datadog/ci/contrib/minitest/runnable.rb

@@ -20,9 +20,10 @@ def run(*)
               test_suite_name = Helpers.test_suite_name(self, method)
 
               test_suite = Datadog::CI.start_test_suite(test_suite_name)
-              test_suite.passed! # will be overridden if any test fails
+              test_suite.passed! if test_suite # will be overridden if any test fails
 
               results = super
+              return results unless test_suite
 
               test_suite.finish
 

lib/datadog/ci/contrib/rspec/example.rb

@@ -41,17 +41,19 @@ def run(*)
                 },
                 service: datadog_configuration[:service_name]
               ) do |test_span|
-                test_span.set_parameters({}, {"scoped_id" => metadata[:scoped_id]})
-
                 result = super
 
-                case execution_result.status
-                when :passed
-                  test_span.passed!
-                when :failed
-                  test_span.failed!(exception: execution_result.exception)
-                else
-                  test_span.skipped!(exception: execution_result.exception) if execution_result.example_skipped?
+                if test_span
+                  test_span.set_parameters({}, {"scoped_id" => metadata[:scoped_id]})
+
+                  case execution_result.status
+                  when :passed
+                    test_span.passed!
+                  when :failed
+                    test_span.failed!(exception: execution_result.exception)
+                  else
+                    test_span.skipped!(exception: execution_result.exception) if execution_result.example_skipped?
+                  end
                 end
 
                 result

lib/datadog/ci/contrib/rspec/example_group.rb

@@ -23,6 +23,7 @@ def run(*)
               test_suite = Datadog::CI.start_test_suite(suite_name)
 
               result = super
+              return result unless test_suite
 
               if result
                 test_suite.passed!

lib/datadog/ci/contrib/rspec/runner.rb

@@ -25,9 +25,10 @@ def run_specs(*)
                 service: datadog_configuration[:service_name]
               )
 
-              test_module = CI.start_test_module(test_session.name)
+              test_module = CI.start_test_module(test_session.name) if test_session
 
               result = super
+              return result unless test_module && test_session
 
               if result != 0
                 # TODO: repeating this twice feels clunky, we need to remove test_module API before GA

lib/datadog/ci/ext/app_types.rb

@@ -3,6 +3,8 @@
 module Datadog
   module CI
     module Ext
+      # Defines span types for CI visibility
+      # @public_api
       module AppTypes
         TYPE_TEST = "test"
         TYPE_TEST_SESSION = "test_session_end"