From 45d5d79dde8c31b45d6b987128db1dd0a400bb9c Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 29 Feb 2024 14:40:57 +0100 Subject: [PATCH 01/20] add DD_CIVISIBILITY_ITR_ENABLED setting --- lib/datadog/ci/configuration/settings.rb | 6 +++ lib/datadog/ci/ext/settings.rb | 1 + sig/datadog/ci/ext/settings.rbs | 1 + .../datadog/ci/configuration/settings_spec.rb | 41 +++++++++++++++++++ 4 files changed, 49 insertions(+) diff --git a/lib/datadog/ci/configuration/settings.rb b/lib/datadog/ci/configuration/settings.rb index 141a6786..2e85a023 100644 --- a/lib/datadog/ci/configuration/settings.rb +++ b/lib/datadog/ci/configuration/settings.rb @@ -55,6 +55,12 @@ def self.add_settings!(base) end end + option :itr_enabled do |o| + o.type :bool + o.env CI::Ext::Settings::ENV_ITR_ENABLED + o.default false + end + define_method(:instrument) do |integration_name, options = {}, &block| return unless enabled diff --git a/lib/datadog/ci/ext/settings.rb b/lib/datadog/ci/ext/settings.rb index 7357d790..a6d16a30 100644 --- a/lib/datadog/ci/ext/settings.rb +++ b/lib/datadog/ci/ext/settings.rb @@ -10,6 +10,7 @@ module Settings ENV_AGENTLESS_URL = "DD_CIVISIBILITY_AGENTLESS_URL" ENV_EXPERIMENTAL_TEST_SUITE_LEVEL_VISIBILITY_ENABLED = "DD_CIVISIBILITY_EXPERIMENTAL_TEST_SUITE_LEVEL_VISIBILITY_ENABLED" ENV_FORCE_TEST_LEVEL_VISIBILITY = "DD_CIVISIBILITY_FORCE_TEST_LEVEL_VISIBILITY" + ENV_ITR_ENABLED = "DD_CIVISIBILITY_ITR_ENABLED" # Source: https://docs.datadoghq.com/getting_started/site/ DD_SITE_ALLOWLIST = [ diff --git a/sig/datadog/ci/ext/settings.rbs b/sig/datadog/ci/ext/settings.rbs index db4fba33..2ead3eab 100644 --- a/sig/datadog/ci/ext/settings.rbs +++ b/sig/datadog/ci/ext/settings.rbs @@ -7,6 +7,7 @@ module Datadog ENV_AGENTLESS_URL: String ENV_EXPERIMENTAL_TEST_SUITE_LEVEL_VISIBILITY_ENABLED: String ENV_FORCE_TEST_LEVEL_VISIBILITY: String + ENV_ITR_ENABLED: String DD_SITE_ALLOWLIST: Array[String] end diff --git a/spec/datadog/ci/configuration/settings_spec.rb b/spec/datadog/ci/configuration/settings_spec.rb index 8139da30..730f0aa4 100644 --- a/spec/datadog/ci/configuration/settings_spec.rb +++ b/spec/datadog/ci/configuration/settings_spec.rb @@ -217,6 +217,47 @@ def patcher end end + describe "#itr_enabled" do + subject(:itr_enabled) { settings.ci.itr_enabled } + + it { is_expected.to be false } + + context "when #{Datadog::CI::Ext::Settings::ENV_ITR_ENABLED}" do + around do |example| + ClimateControl.modify(Datadog::CI::Ext::Settings::ENV_ITR_ENABLED => enable) do + example.run + end + end + + context "is not defined" do + let(:enable) { nil } + + it { is_expected.to be false } + end + + context "is set to true" do + let(:enable) { "true" } + + it { is_expected.to be true } + end + + context "is set to false" do + let(:enable) { "false" } + + it { is_expected.to be false } + end + end + end + + describe "#itr_enabled=" do + it "updates the #enabled setting" do + expect { settings.ci.itr_enabled = true } + .to change { settings.ci.itr_enabled } + .from(false) + .to(true) + end + end + describe "#instrument" do let(:integration_name) { :fake } From 1169f664079904bd37034021031c396a96b47f20 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 29 Feb 2024 14:58:20 +0100 Subject: [PATCH 02/20] add ITR::Runner class --- lib/datadog/ci/itr/runner.rb | 28 +++++++++++++++++++ sig/datadog/ci/itr/runner.rbs | 15 ++++++++++ spec/datadog/ci/itr/runner_spec.rb | 28 +++++++++++++++++++ .../ci/test_visibility/recorder_spec.rb | 4 +++ 4 files changed, 75 insertions(+) create mode 100644 lib/datadog/ci/itr/runner.rb create mode 100644 sig/datadog/ci/itr/runner.rbs create mode 100644 spec/datadog/ci/itr/runner_spec.rb diff --git a/lib/datadog/ci/itr/runner.rb b/lib/datadog/ci/itr/runner.rb new file mode 100644 index 00000000..0e3269b5 --- /dev/null +++ b/lib/datadog/ci/itr/runner.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Datadog + module CI + module ITR + # Intelligent test runner implementation + # Integrates with backend to provide test impact analysis data and + # skip tests that are not impacted by the changes + class Runner + def initialize( + enabled: false + ) + @enabled = enabled + end + + def enabled? + @enabled + end + + def configure + return unless enabled? + + Datadog.logger.debug("Sending ITR settings request...") + end + end + end + end +end diff --git a/sig/datadog/ci/itr/runner.rbs b/sig/datadog/ci/itr/runner.rbs new file mode 100644 index 00000000..5bb52120 --- /dev/null +++ b/sig/datadog/ci/itr/runner.rbs @@ -0,0 +1,15 @@ +module Datadog + module CI + module ITR + class Runner + @enabled: bool + + def initialize: (?enabled: bool) -> void + + def enabled?: () -> bool + + def configure: () -> void + end + end + end +end diff --git a/spec/datadog/ci/itr/runner_spec.rb b/spec/datadog/ci/itr/runner_spec.rb new file mode 100644 index 00000000..f29f2f5e --- /dev/null +++ b/spec/datadog/ci/itr/runner_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require_relative "../../../../lib/datadog/ci/itr/runner" + +RSpec.describe Datadog::CI::ITR::Runner do + let(:itr_enabled) { true } + subject(:runner) { described_class.new(enabled: itr_enabled) } + + describe "#configure" do + context "itr enabled" do + before do + expect(Datadog.logger).to receive(:debug).with("Sending ITR settings request...") + end + + it { runner.configure } + end + + context "itr disabled" do + let(:itr_enabled) { false } + + before do + expect(Datadog.logger).to_not receive(:debug).with("Sending ITR settings request...") + end + + it { runner.configure } + end + end +end diff --git a/spec/datadog/ci/test_visibility/recorder_spec.rb b/spec/datadog/ci/test_visibility/recorder_spec.rb index 7232d36e..4f074867 100644 --- a/spec/datadog/ci/test_visibility/recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/recorder_spec.rb @@ -1,3 +1,7 @@ +# frozen_string_literal: true + +require_relative "../../../../lib/datadog/ci/test_visibility/recorder" + RSpec.describe Datadog::CI::TestVisibility::Recorder do shared_examples_for "trace with ciapp-test origin" do let(:trace_under_test) { subject } From 7d5bb6fd1886022e69d8308ec25425278d2bcd00 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 29 Feb 2024 15:48:21 +0100 Subject: [PATCH 03/20] configure ITR runner when instantiating the CI component --- lib/datadog/ci/configuration/components.rb | 16 +++++++--- lib/datadog/ci/test_visibility/recorder.rb | 7 ++++- sig/datadog/ci/test_visibility/recorder.rbs | 3 +- .../ci/configuration/components_spec.rb | 29 +++++++++++++++++++ .../ci/test_visibility/recorder_spec.rb | 10 +++++++ 5 files changed, 59 insertions(+), 6 deletions(-) diff --git a/lib/datadog/ci/configuration/components.rb b/lib/datadog/ci/configuration/components.rb index 1ce8e2c3..6db4e023 100644 --- a/lib/datadog/ci/configuration/components.rb +++ b/lib/datadog/ci/configuration/components.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative "../ext/settings" +require_relative "../itr/runner" require_relative "../test_visibility/flush" require_relative "../test_visibility/recorder" require_relative "../test_visibility/null_recorder" @@ -59,13 +60,23 @@ def activate_ci!(settings) writer_options[:buffer_size] = 10_000 settings.tracing.test_mode.async = true + else + # only legacy APM protocol is supported, so no test suite level visibility + settings.ci.force_test_level_visibility = true + + # ITR is not supported with APM protocol + settings.ci.itr_enabled = false end settings.tracing.test_mode.writer_options = writer_options + itr = Datadog::CI::ITR::Runner.new(enabled: settings.ci.itr_enabled) + itr.configure + # CI visibility recorder global instance @ci_recorder = TestVisibility::Recorder.new( - test_suite_level_visibility_enabled: !settings.ci.force_test_level_visibility + test_suite_level_visibility_enabled: !settings.ci.force_test_level_visibility, + itr: itr ) end @@ -95,9 +106,6 @@ def build_test_visibility_api(settings) Datadog.logger.debug( "Old agent version detected, no evp_proxy support. Forcing test level visibility mode" ) - - # CI visibility is still enabled but in legacy test level visibility mode - settings.ci.force_test_level_visibility = true end end diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index c274590f..21c1103a 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -30,14 +30,19 @@ class Recorder def initialize( test_suite_level_visibility_enabled: false, - codeowners: Codeowners::Parser.new(Utils::Git.root).parse + codeowners: Codeowners::Parser.new(Utils::Git.root).parse, + itr: nil ) @test_suite_level_visibility_enabled = test_suite_level_visibility_enabled @environment_tags = Ext::Environment.tags(ENV).freeze @local_context = Context::Local.new @global_context = Context::Global.new + @codeowners = codeowners + + raise ArgumentError, "ITR runner is required" unless itr + @itr = itr end def start_test_session(service: nil, tags: {}) diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index 490f7302..732d5f08 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -7,12 +7,13 @@ module Datadog @environment_tags: Hash[String, String] @local_context: Datadog::CI::TestVisibility::Context::Local @global_context: Datadog::CI::TestVisibility::Context::Global + @itr: Datadog::CI::ITR::Runner @codeowners: Datadog::CI::Codeowners::Matcher attr_reader environment_tags: Hash[String, String] attr_reader test_suite_level_visibility_enabled: bool - def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher) -> void + def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher, ?itr: Datadog::CI::ITR::Runner?) -> void def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Test span) -> untyped } -> untyped diff --git a/spec/datadog/ci/configuration/components_spec.rb b/spec/datadog/ci/configuration/components_spec.rb index bf10ac1e..587a1e4b 100644 --- a/spec/datadog/ci/configuration/components_spec.rb +++ b/spec/datadog/ci/configuration/components_spec.rb @@ -1,3 +1,7 @@ +# frozen_string_literal: true + +require_relative "../../../../lib/datadog/ci/configuration/components" + RSpec.describe Datadog::CI::Configuration::Components do context "when used to extend Datadog::Core::Configuration::Components" do subject(:components) do @@ -50,6 +54,10 @@ .to receive(:agentless_url) .and_return(agentless_url) + allow(settings.ci) + .to receive(:itr_enabled) + .and_return(itr_enabled) + allow(settings) .to receive(:site) .and_return(dd_site) @@ -91,6 +99,12 @@ allow(settings.ci) .to receive(:force_test_level_visibility=) + allow(settings.ci) + .to receive(:itr_enabled=) + + allow(Datadog.logger) + .to receive(:debug) + allow(Datadog.logger) .to receive(:warn) @@ -110,6 +124,7 @@ let(:force_test_level_visibility) { false } let(:evp_proxy_v2_supported) { false } let(:evp_proxy_v4_supported) { false } + let(:itr_enabled) { false } context "is enabled" do let(:enabled) { true } @@ -186,6 +201,10 @@ .to have_received(:force_test_level_visibility=) .with(true) + expect(settings.ci) + .to have_received(:itr_enabled=) + .with(false) + expect(settings.tracing.test_mode).to have_received(:writer_options=) do |options| expect(options[:transport]).to be_nil end @@ -245,6 +264,16 @@ end end end + + context "and when itr" do + context "is enabled" do + let(:itr_enabled) { true } + + it "configures ITR" do + expect(Datadog.logger).to have_received(:debug).with("Sending ITR settings request...") + end + end + end end context "is disabled" do diff --git a/spec/datadog/ci/test_visibility/recorder_spec.rb b/spec/datadog/ci/test_visibility/recorder_spec.rb index 4f074867..d8a0c1b6 100644 --- a/spec/datadog/ci/test_visibility/recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/recorder_spec.rb @@ -48,6 +48,16 @@ end end + describe "#initialize" do + context "no ITR runner is provided" do + subject { described_class.new } + + it "raises an error" do + expect { subject }.to raise_error(ArgumentError, "ITR runner is required") + end + end + end + context "when test suite level visibility is disabled" do let(:service) { "my-service" } let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } From 500088663e99e3344884cf25967bb57fea1a33e1 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Fri, 1 Mar 2024 15:23:33 +0100 Subject: [PATCH 04/20] add ITR client to communicate with CI vis backend --- lib/datadog/ci/configuration/components.rb | 6 +- lib/datadog/ci/ext/itr.rb | 12 ++ lib/datadog/ci/itr/client.rb | 43 ++++++ lib/datadog/ci/itr/runner.rb | 14 +- lib/datadog/ci/test_visibility/recorder.rb | 8 ++ sig/datadog/ci/ext/itr.rbs | 9 ++ sig/datadog/ci/itr/client.rbs | 17 +++ sig/datadog/ci/itr/runner.rbs | 4 +- sig/datadog/ci/test_visibility/recorder.rbs | 2 + .../ci/configuration/components_spec.rb | 126 ++++++++---------- spec/datadog/ci/itr/runner_spec.rb | 22 ++- 11 files changed, 179 insertions(+), 84 deletions(-) create mode 100644 lib/datadog/ci/ext/itr.rb create mode 100644 lib/datadog/ci/itr/client.rb create mode 100644 sig/datadog/ci/ext/itr.rbs create mode 100644 sig/datadog/ci/itr/client.rbs diff --git a/lib/datadog/ci/configuration/components.rb b/lib/datadog/ci/configuration/components.rb index 6db4e023..4c111d0d 100644 --- a/lib/datadog/ci/configuration/components.rb +++ b/lib/datadog/ci/configuration/components.rb @@ -70,8 +70,10 @@ def activate_ci!(settings) settings.tracing.test_mode.writer_options = writer_options - itr = Datadog::CI::ITR::Runner.new(enabled: settings.ci.itr_enabled) - itr.configure + itr = Datadog::CI::ITR::Runner.new( + enabled: settings.ci.enabled && settings.ci.itr_enabled, + api: test_visibility_api + ) # CI visibility recorder global instance @ci_recorder = TestVisibility::Recorder.new( diff --git a/lib/datadog/ci/ext/itr.rb b/lib/datadog/ci/ext/itr.rb new file mode 100644 index 00000000..5a5ad0b1 --- /dev/null +++ b/lib/datadog/ci/ext/itr.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Datadog + module CI + module Ext + # Defines constants for Git tags + module ITR + API_TYPE_SETTINGS = "ci_app_test_service_libraries_settings" + end + end + end +end diff --git a/lib/datadog/ci/itr/client.rb b/lib/datadog/ci/itr/client.rb new file mode 100644 index 00000000..c494d390 --- /dev/null +++ b/lib/datadog/ci/itr/client.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require_relative "../ext/itr" + +module Datadog + module CI + module ITR + # ITR API client + # communicates with the CI visibility backend + class Client + def initialize(api: nil) + raise ArgumentError, "Test visibility API is required" unless api + + @api = api + end + + def fetch_settings(service:) + # TODO: application/json support + # TODO: runtime information is required for payload + # TODO: return error response - use some wrapper from ddtrace as an example + @api.request( + path: "/api/v2/ci/libraries/tests/services/setting", + payload: settings_payload(service: service) + ) + end + + private + + def settings_payload(service:) + { + data: { + id: "change_me", + type: Ext::ITR::API_TYPE_SETTINGS, + attributes: { + service: service + } + } + }.to_json + end + end + end + end +end diff --git a/lib/datadog/ci/itr/runner.rb b/lib/datadog/ci/itr/runner.rb index 0e3269b5..651ed8c2 100644 --- a/lib/datadog/ci/itr/runner.rb +++ b/lib/datadog/ci/itr/runner.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "client" + module Datadog module CI module ITR @@ -8,19 +10,25 @@ module ITR # skip tests that are not impacted by the changes class Runner def initialize( - enabled: false + enabled: false, + api: nil ) @enabled = enabled + return unless enabled + + @client = Client.new(api: api) end def enabled? @enabled end - def configure + def configure(service:) return unless enabled? - Datadog.logger.debug("Sending ITR settings request...") + # TODO: error handling when request failed + # TODO: need to pass runtime information + @client.fetch_settings(service: service) end end end diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index 21c1103a..7b45594c 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -48,6 +48,10 @@ def initialize( def start_test_session(service: nil, tags: {}) return skip_tracing unless test_suite_level_visibility_enabled + # TODO: pass runtime and environment information + # actually this is the correct place to configure ITR - when starting test session + @itr.configure(service: service) + @global_context.fetch_or_activate_test_session do tracer_span = start_datadog_tracer_span( "test.session", build_span_options(service, Ext::AppTypes::TYPE_TEST_SESSION) @@ -180,6 +184,10 @@ def deactivate_test_suite(test_suite_name) @global_context.deactivate_test_suite!(test_suite_name) end + def itr_enabled? + @itr.enabled? + end + private def skip_tracing(block = nil) diff --git a/sig/datadog/ci/ext/itr.rbs b/sig/datadog/ci/ext/itr.rbs new file mode 100644 index 00000000..bf48b547 --- /dev/null +++ b/sig/datadog/ci/ext/itr.rbs @@ -0,0 +1,9 @@ +module Datadog + module CI + module Ext + module ITR + API_TYPE_SETTINGS: "ci_app_test_service_libraries_settings" + end + end + end +end diff --git a/sig/datadog/ci/itr/client.rbs b/sig/datadog/ci/itr/client.rbs new file mode 100644 index 00000000..311ca3ef --- /dev/null +++ b/sig/datadog/ci/itr/client.rbs @@ -0,0 +1,17 @@ +module Datadog + module CI + module ITR + class Client + @api: untyped + + def initialize: (?api: Datadog::CI::Transport::Api::Base?) -> void + + def fetch_settings: (service: String?) -> untyped + + private + + def settings_payload: (service: String?) -> String + end + end + end +end diff --git a/sig/datadog/ci/itr/runner.rbs b/sig/datadog/ci/itr/runner.rbs index 5bb52120..21d915fd 100644 --- a/sig/datadog/ci/itr/runner.rbs +++ b/sig/datadog/ci/itr/runner.rbs @@ -4,11 +4,11 @@ module Datadog class Runner @enabled: bool - def initialize: (?enabled: bool) -> void + def initialize: (?enabled: bool, api: Datadog::CI::Transport::Api::Base?) -> void def enabled?: () -> bool - def configure: () -> void + def configure: (service: String?) -> void end end end diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index 732d5f08..130bbb37 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -43,6 +43,8 @@ module Datadog def deactivate_test_suite: (String test_suite_name) -> void + def itr_enabled?: () -> bool + private def create_datadog_span: (String span_name, ?span_options: Hash[untyped, untyped], ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped diff --git a/spec/datadog/ci/configuration/components_spec.rb b/spec/datadog/ci/configuration/components_spec.rb index 587a1e4b..62e00e90 100644 --- a/spec/datadog/ci/configuration/components_spec.rb +++ b/spec/datadog/ci/configuration/components_spec.rb @@ -37,34 +37,15 @@ describe "::new" do context "when #ci" do before do - # Stub CI mode behavior - allow(settings.ci) - .to receive(:enabled) - .and_return(enabled) + # Configure CI mode + settings.ci.enabled = enabled + settings.ci.agentless_mode_enabled = agentless_enabled - allow(settings.ci) - .to receive(:agentless_mode_enabled) - .and_return(agentless_enabled) - - allow(settings.ci) - .to receive(:force_test_level_visibility) - .and_return(force_test_level_visibility) - - allow(settings.ci) - .to receive(:agentless_url) - .and_return(agentless_url) - - allow(settings.ci) - .to receive(:itr_enabled) - .and_return(itr_enabled) - - allow(settings) - .to receive(:site) - .and_return(dd_site) - - allow(settings) - .to receive(:api_key) - .and_return(api_key) + settings.ci.force_test_level_visibility = force_test_level_visibility + settings.ci.agentless_url = agentless_url + settings.ci.itr_enabled = itr_enabled + settings.site = dd_site + settings.api_key = api_key negotiation = double(:negotiation) @@ -82,25 +63,16 @@ # Spy on test mode behavior allow(settings.tracing.test_mode) - .to receive(:enabled=) + .to receive(:enabled=).and_call_original allow(settings.tracing.test_mode) - .to receive(:trace_flush=) + .to receive(:trace_flush=).and_call_original allow(settings.tracing.test_mode) - .to receive(:writer_options=) + .to receive(:writer_options=).and_call_original allow(settings.tracing.test_mode) - .to receive(:async=) - - allow(settings.ci) - .to receive(:enabled=) - - allow(settings.ci) - .to receive(:force_test_level_visibility=) - - allow(settings.ci) - .to receive(:itr_enabled=) + .to receive(:async=).and_call_original allow(Datadog.logger) .to receive(:debug) @@ -134,6 +106,8 @@ end context "when #force_test_level_visibility" do + let(:evp_proxy_v2_supported) { true } + context "is false" do it "creates a CI recorder with test_suite_level_visibility_enabled=true" do expect(components.ci_recorder).to be_kind_of(Datadog::CI::TestVisibility::Recorder) @@ -188,7 +162,9 @@ end context "and when agent does not support EVP proxy" do - it "falls back to default transport and disables test suite level visibility" do + let(:itr_enabled) { true } + + it "falls back to default transport and disables test suite level visibility and ITR" do expect(settings.tracing.test_mode) .to have_received(:enabled=) .with(true) @@ -197,17 +173,14 @@ .to have_received(:trace_flush=) .with(settings.ci.trace_flush || kind_of(Datadog::CI::TestVisibility::Flush::Partial)) - expect(settings.ci) - .to have_received(:force_test_level_visibility=) - .with(true) - - expect(settings.ci) - .to have_received(:itr_enabled=) - .with(false) + expect(settings.ci.force_test_level_visibility).to eq(true) + expect(settings.ci.itr_enabled).to eq(false) expect(settings.tracing.test_mode).to have_received(:writer_options=) do |options| expect(options[:transport]).to be_nil end + + expect(components.ci_recorder.itr_enabled?).to eq(false) end end end @@ -232,45 +205,54 @@ expect(options[:shutdown_timeout]).to eq(60) end end - end - context "when DD_SITE is set to a wrong value" do - let(:dd_site) { "wrong" } + context "when DD_SITE is set to a wrong value" do + let(:dd_site) { "wrong" } - it "logs a warning" do - expect(Datadog.logger).to have_received(:warn) do |*_args, &block| - expect(block.call).to match( - /CI VISIBILITY CONFIGURATION Agentless mode was enabled but DD_SITE is not set to one of the following/ - ) + it "logs a warning" do + expect(Datadog.logger).to have_received(:warn) do |*_args, &block| + expect(block.call).to match( + /CI VISIBILITY CONFIGURATION Agentless mode was enabled but DD_SITE is not set to one of the following/ + ) + end end end - end - context "when DD_SITE is set to a correct value" do - let(:dd_site) { "datadoghq.eu" } + context "when DD_SITE is set to a correct value" do + let(:dd_site) { "datadoghq.eu" } - it "logs a warning" do - expect(Datadog.logger).not_to have_received(:warn) + it "does not log a warning" do + expect(Datadog.logger).not_to have_received(:warn) + end + end + + context "when ITR is disabled" do + let(:itr_enabled) { false } + + it "creates a CI recorder with ITR disabled" do + expect(components.ci_recorder.itr_enabled?).to eq(false) + end + end + + context "when ITR is enabled" do + let(:itr_enabled) { true } + + it "creates a CI recorder with ITR enabled" do + expect(components.ci_recorder.itr_enabled?).to eq(true) + end end end context "when api key is not set" do let(:api_key) { nil } + let(:itr_enabled) { true } it "logs an error message and disables CI visibility" do expect(Datadog.logger).to have_received(:error) - expect(settings.ci).to have_received(:enabled=).with(false) - end - end - end - end - - context "and when itr" do - context "is enabled" do - let(:itr_enabled) { true } - it "configures ITR" do - expect(Datadog.logger).to have_received(:debug).with("Sending ITR settings request...") + expect(settings.ci.enabled).to eq(false) + expect(components.ci_recorder.itr_enabled?).to eq(false) + end end end end diff --git a/spec/datadog/ci/itr/runner_spec.rb b/spec/datadog/ci/itr/runner_spec.rb index f29f2f5e..63839eba 100644 --- a/spec/datadog/ci/itr/runner_spec.rb +++ b/spec/datadog/ci/itr/runner_spec.rb @@ -4,25 +4,37 @@ RSpec.describe Datadog::CI::ITR::Runner do let(:itr_enabled) { true } - subject(:runner) { described_class.new(enabled: itr_enabled) } + + let(:api) { double("api") } + let(:client) { double("client") } + + subject(:runner) { described_class.new(enabled: itr_enabled, api: api) } describe "#configure" do + let(:service) { "service" } + context "itr enabled" do before do - expect(Datadog.logger).to receive(:debug).with("Sending ITR settings request...") + expect(Datadog::CI::ITR::Client).to receive(:new).with(api: api).and_return(client) end - it { runner.configure } + it "fetches settings from backend" do + expect(client).to receive(:fetch_settings).with(service: service) + + runner.configure(service: service) + end end context "itr disabled" do let(:itr_enabled) { false } before do - expect(Datadog.logger).to_not receive(:debug).with("Sending ITR settings request...") + expect(Datadog::CI::ITR::Client).to_not receive(:new) end - it { runner.configure } + it "does nothing" do + expect(runner.configure(service: service)).to be_nil + end end end end From b90c6d11938ac79377564222a82eb46446aacb47 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Fri, 1 Mar 2024 16:13:29 +0100 Subject: [PATCH 05/20] support headers override for test visibility API --- lib/datadog/ci/ext/itr.rb | 2 ++ lib/datadog/ci/ext/transport.rb | 1 + lib/datadog/ci/itr/client.rb | 6 ++-- lib/datadog/ci/transport/api/agentless.rb | 2 +- lib/datadog/ci/transport/api/base.rb | 9 +++-- lib/datadog/ci/transport/api/evp_proxy.rb | 5 +-- sig/datadog/ci/ext/itr.rbs | 2 ++ sig/datadog/ci/ext/transport.rbs | 2 ++ sig/datadog/ci/itr/runner.rbs | 1 + sig/datadog/ci/transport/api/agentless.rbs | 4 +-- sig/datadog/ci/transport/api/base.rbs | 4 +-- sig/datadog/ci/transport/api/evp_proxy.rbs | 4 +-- spec/datadog/ci/itr/client_spec.rb | 33 +++++++++++++++++++ spec/datadog/ci/itr/runner_spec.rb | 1 + .../ci/transport/api/agentless_spec.rb | 14 ++++++++ .../ci/transport/api/evp_proxy_spec.rb | 16 +++++++++ 16 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 spec/datadog/ci/itr/client_spec.rb diff --git a/lib/datadog/ci/ext/itr.rb b/lib/datadog/ci/ext/itr.rb index 5a5ad0b1..d93a8501 100644 --- a/lib/datadog/ci/ext/itr.rb +++ b/lib/datadog/ci/ext/itr.rb @@ -6,6 +6,8 @@ module Ext # Defines constants for Git tags module ITR API_TYPE_SETTINGS = "ci_app_test_service_libraries_settings" + + API_PATH_SETTINGS = "/api/v2/ci/libraries/tests/services/setting" end end end diff --git a/lib/datadog/ci/ext/transport.rb b/lib/datadog/ci/ext/transport.rb index d7da4acf..509b1fba 100644 --- a/lib/datadog/ci/ext/transport.rb +++ b/lib/datadog/ci/ext/transport.rb @@ -24,6 +24,7 @@ module Transport TEST_VISIBILITY_INTAKE_PATH = "/api/v2/citestcycle" CONTENT_TYPE_MESSAGEPACK = "application/msgpack" + CONTENT_TYPE_JSON = "application/json" CONTENT_ENCODING_GZIP = "gzip" end end diff --git a/lib/datadog/ci/itr/client.rb b/lib/datadog/ci/itr/client.rb index c494d390..f8f19693 100644 --- a/lib/datadog/ci/itr/client.rb +++ b/lib/datadog/ci/itr/client.rb @@ -16,11 +16,13 @@ def initialize(api: nil) def fetch_settings(service:) # TODO: application/json support + # TODO: id generation # TODO: runtime information is required for payload # TODO: return error response - use some wrapper from ddtrace as an example @api.request( - path: "/api/v2/ci/libraries/tests/services/setting", - payload: settings_payload(service: service) + path: Ext::ITR::API_PATH_SETTINGS, + payload: settings_payload(service: service), + headers: {Ext::Transport::HEADER_CONTENT_TYPE => Ext::Transport::CONTENT_TYPE_JSON} ) end diff --git a/lib/datadog/ci/transport/api/agentless.rb b/lib/datadog/ci/transport/api/agentless.rb index 3e4503f6..8e1b6bf4 100644 --- a/lib/datadog/ci/transport/api/agentless.rb +++ b/lib/datadog/ci/transport/api/agentless.rb @@ -18,7 +18,7 @@ def initialize(api_key:, http:) private - def headers + def default_headers headers = super headers[Ext::Transport::HEADER_DD_API_KEY] = api_key headers diff --git a/lib/datadog/ci/transport/api/base.rb b/lib/datadog/ci/transport/api/base.rb index 6230b887..9e9f053c 100644 --- a/lib/datadog/ci/transport/api/base.rb +++ b/lib/datadog/ci/transport/api/base.rb @@ -13,18 +13,21 @@ def initialize(http:) @http = http end - def request(path:, payload:, verb: "post") + def request(path:, payload:, headers: {}, verb: "post") + request_headers = default_headers + request_headers.merge!(headers) + http.request( path: path, payload: payload, verb: verb, - headers: headers + headers: request_headers ) end private - def headers + def default_headers { Ext::Transport::HEADER_CONTENT_TYPE => Ext::Transport::CONTENT_TYPE_MESSAGEPACK } diff --git a/lib/datadog/ci/transport/api/evp_proxy.rb b/lib/datadog/ci/transport/api/evp_proxy.rb index 3e7067a2..05d18675 100644 --- a/lib/datadog/ci/transport/api/evp_proxy.rb +++ b/lib/datadog/ci/transport/api/evp_proxy.rb @@ -17,12 +17,13 @@ def initialize(http:, path_prefix: Ext::Transport::EVP_PROXY_V2_PATH_PREFIX) @path_prefix = path_prefix end - def request(path:, payload:, verb: "post") + def request(path:, payload:, headers: {}, verb: "post") path = "#{@path_prefix}#{path.sub(/^\//, "")}" super( path: path, payload: payload, + headers: headers, verb: verb ) end @@ -35,7 +36,7 @@ def container_id @container_id = Datadog::Core::Environment::Container.container_id end - def headers + def default_headers headers = super headers[Ext::Transport::HEADER_EVP_SUBDOMAIN] = Ext::Transport::TEST_VISIBILITY_INTAKE_HOST_PREFIX diff --git a/sig/datadog/ci/ext/itr.rbs b/sig/datadog/ci/ext/itr.rbs index bf48b547..9296de59 100644 --- a/sig/datadog/ci/ext/itr.rbs +++ b/sig/datadog/ci/ext/itr.rbs @@ -3,6 +3,8 @@ module Datadog module Ext module ITR API_TYPE_SETTINGS: "ci_app_test_service_libraries_settings" + + API_PATH_SETTINGS: "/api/v2/ci/libraries/tests/services/setting" end end end diff --git a/sig/datadog/ci/ext/transport.rbs b/sig/datadog/ci/ext/transport.rbs index a52918f7..cfbd6511 100644 --- a/sig/datadog/ci/ext/transport.rbs +++ b/sig/datadog/ci/ext/transport.rbs @@ -28,6 +28,8 @@ module Datadog CONTENT_TYPE_MESSAGEPACK: "application/msgpack" + CONTENT_TYPE_JSON: "application/json" + CONTENT_ENCODING_GZIP: "gzip" end end diff --git a/sig/datadog/ci/itr/runner.rbs b/sig/datadog/ci/itr/runner.rbs index 21d915fd..5155a00a 100644 --- a/sig/datadog/ci/itr/runner.rbs +++ b/sig/datadog/ci/itr/runner.rbs @@ -3,6 +3,7 @@ module Datadog module ITR class Runner @enabled: bool + @client: Datadog::CI::ITR::Client def initialize: (?enabled: bool, api: Datadog::CI::Transport::Api::Base?) -> void diff --git a/sig/datadog/ci/transport/api/agentless.rbs b/sig/datadog/ci/transport/api/agentless.rbs index cbe63b14..bbc7f206 100644 --- a/sig/datadog/ci/transport/api/agentless.rbs +++ b/sig/datadog/ci/transport/api/agentless.rbs @@ -9,11 +9,11 @@ module Datadog def initialize: (api_key: String, http: Datadog::CI::Transport::HTTP) -> void - def request: (path: String, payload: String, ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator + def request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator private - def headers: () -> Hash[String, String] + def default_headers: () -> Hash[String, String] end end end diff --git a/sig/datadog/ci/transport/api/base.rbs b/sig/datadog/ci/transport/api/base.rbs index 6fd14d70..c2f5b458 100644 --- a/sig/datadog/ci/transport/api/base.rbs +++ b/sig/datadog/ci/transport/api/base.rbs @@ -9,11 +9,11 @@ module Datadog def initialize: (http: Datadog::CI::Transport::HTTP) -> void - def request: (path: String, payload: String, ?verb: ::String) -> untyped + def request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> untyped private - def headers: () -> Hash[String, String] + def default_headers: () -> Hash[String, String] end end end diff --git a/sig/datadog/ci/transport/api/evp_proxy.rbs b/sig/datadog/ci/transport/api/evp_proxy.rbs index 7f1a5221..34e064ff 100644 --- a/sig/datadog/ci/transport/api/evp_proxy.rbs +++ b/sig/datadog/ci/transport/api/evp_proxy.rbs @@ -8,13 +8,13 @@ module Datadog def initialize: (http: Datadog::CI::Transport::HTTP, ?path_prefix: String) -> void - def request: (path: String, payload: String, ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator + def request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator private def container_id: () -> String? - def headers: () -> Hash[String, String] + def default_headers: () -> Hash[String, String] end end end diff --git a/spec/datadog/ci/itr/client_spec.rb b/spec/datadog/ci/itr/client_spec.rb new file mode 100644 index 00000000..9ab3cdb5 --- /dev/null +++ b/spec/datadog/ci/itr/client_spec.rb @@ -0,0 +1,33 @@ +require_relative "../../../../lib/datadog/ci/itr/client" +require_relative "../../../../lib/datadog/ci/ext/itr" + +RSpec.describe Datadog::CI::ITR::Client do + let(:api) { spy("api") } + subject { described_class.new(api: api) } + + describe "#fetch_settings" do + let(:service) { "service" } + let(:path) { Datadog::CI::Ext::ITR::API_PATH_SETTINGS } + let(:payload) do + { + data: { + id: "change_me", + type: Datadog::CI::Ext::ITR::API_TYPE_SETTINGS, + attributes: { + service: service + } + } + }.to_json + end + + it "requests the settings" do + subject.fetch_settings(service: service) + + expect(api).to have_received(:request).with( + path: path, + payload: payload, + headers: {"Content-Type" => "application/json"} + ) + end + end +end diff --git a/spec/datadog/ci/itr/runner_spec.rb b/spec/datadog/ci/itr/runner_spec.rb index 63839eba..77b88447 100644 --- a/spec/datadog/ci/itr/runner_spec.rb +++ b/spec/datadog/ci/itr/runner_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require_relative "../../../../lib/datadog/ci/itr/client" require_relative "../../../../lib/datadog/ci/itr/runner" RSpec.describe Datadog::CI::ITR::Runner do diff --git a/spec/datadog/ci/transport/api/agentless_spec.rb b/spec/datadog/ci/transport/api/agentless_spec.rb index cfeb413c..7f11848c 100644 --- a/spec/datadog/ci/transport/api/agentless_spec.rb +++ b/spec/datadog/ci/transport/api/agentless_spec.rb @@ -29,5 +29,19 @@ subject.request(path: "path", payload: "payload") end + + it "alows to override headers" do + expect(http).to receive(:request).with( + path: "path", + payload: "payload", + verb: "post", + headers: { + "DD-API-KEY" => "api_key", + "Content-Type" => "application/json" + } + ) + + subject.request(path: "path", payload: "payload", headers: {"Content-Type" => "application/json"}) + end end end diff --git a/spec/datadog/ci/transport/api/evp_proxy_spec.rb b/spec/datadog/ci/transport/api/evp_proxy_spec.rb index 94d0571f..e7b7d531 100644 --- a/spec/datadog/ci/transport/api/evp_proxy_spec.rb +++ b/spec/datadog/ci/transport/api/evp_proxy_spec.rb @@ -84,5 +84,21 @@ subject.request(path: "/path", payload: "payload") end end + + context "overriding content-type" do + it "uses content type header from the request parameter" do + expect(http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: { + "Content-Type" => "application/json", + "X-Datadog-EVP-Subdomain" => "citestcycle-intake" + } + ) + + subject.request(path: "/path", payload: "payload", headers: {"Content-Type" => "application/json"}) + end + end end end From 1f6d3cbeabd6abbb27ea85f87978905b8b6519f3 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Mon, 4 Mar 2024 15:06:25 +0100 Subject: [PATCH 06/20] API layer now supports both api.datadoghq.com and citestcycle-intake.datadoghq.com --- lib/datadog/ci/ext/itr.rb | 1 - lib/datadog/ci/ext/transport.rb | 4 + lib/datadog/ci/test_visibility/transport.rb | 2 +- lib/datadog/ci/transport/api/agentless.rb | 37 ++- lib/datadog/ci/transport/api/base.rb | 21 +- lib/datadog/ci/transport/api/builder.rb | 26 +- lib/datadog/ci/transport/api/evp_proxy.rb | 50 +++- sig/datadog/ci/ext/transport.rbs | 6 + sig/datadog/ci/transport/api/agentless.rbs | 8 +- sig/datadog/ci/transport/api/base.rbs | 10 +- sig/datadog/ci/transport/api/evp_proxy.rbs | 14 +- .../ci/test_visibility/transport_spec.rb | 14 +- .../ci/transport/api/agentless_spec.rb | 137 +++++++++-- spec/datadog/ci/transport/api/builder_spec.rb | 49 +--- .../ci/transport/api/evp_proxy_spec.rb | 227 +++++++++++++----- 15 files changed, 414 insertions(+), 192 deletions(-) diff --git a/lib/datadog/ci/ext/itr.rb b/lib/datadog/ci/ext/itr.rb index d93a8501..48ed8485 100644 --- a/lib/datadog/ci/ext/itr.rb +++ b/lib/datadog/ci/ext/itr.rb @@ -3,7 +3,6 @@ module Datadog module CI module Ext - # Defines constants for Git tags module ITR API_TYPE_SETTINGS = "ci_app_test_service_libraries_settings" diff --git a/lib/datadog/ci/ext/transport.rb b/lib/datadog/ci/ext/transport.rb index 509b1fba..93d34f6e 100644 --- a/lib/datadog/ci/ext/transport.rb +++ b/lib/datadog/ci/ext/transport.rb @@ -23,6 +23,10 @@ module Transport TEST_VISIBILITY_INTAKE_HOST_PREFIX = "citestcycle-intake" TEST_VISIBILITY_INTAKE_PATH = "/api/v2/citestcycle" + DD_API_HOST_PREFIX = "api" + DD_API_SETTINGS_PATH = "/api/v2/ci/libraries/tests/services/setting" + DD_API_SETTINGS_TYPE = "ci_app_test_service_libraries_settings" + CONTENT_TYPE_MESSAGEPACK = "application/msgpack" CONTENT_TYPE_JSON = "application/json" CONTENT_ENCODING_GZIP = "gzip" diff --git a/lib/datadog/ci/test_visibility/transport.rb b/lib/datadog/ci/test_visibility/transport.rb index cd0d17f3..afc2aa77 100644 --- a/lib/datadog/ci/test_visibility/transport.rb +++ b/lib/datadog/ci/test_visibility/transport.rb @@ -68,7 +68,7 @@ def send_traces(traces) private def send_payload(encoded_payload) - api.request( + api.citestcycle_request( path: Datadog::CI::Ext::Transport::TEST_VISIBILITY_INTAKE_PATH, payload: encoded_payload ) diff --git a/lib/datadog/ci/transport/api/agentless.rb b/lib/datadog/ci/transport/api/agentless.rb index 8e1b6bf4..fb6ad7ac 100644 --- a/lib/datadog/ci/transport/api/agentless.rb +++ b/lib/datadog/ci/transport/api/agentless.rb @@ -10,14 +10,47 @@ module Api class Agentless < Base attr_reader :api_key - def initialize(api_key:, http:) + def initialize(api_key:, citestcycle_url:, api_url:) @api_key = api_key + @citestcycle_http = build_http_client(citestcycle_url, compress: true) + @api_http = build_http_client(api_url, compress: false) + end + + def citestcycle_request(path:, payload:, headers: {}, verb: "post") + super + + perform_request(@citestcycle_http, path: path, payload: payload, headers: headers, verb: verb) + end + + def api_request(path:, payload:, headers: {}, verb: "post") + super - super(http: http) + perform_request(@api_http, path: path, payload: payload, headers: headers, verb: verb) end private + def perform_request(http_client, path:, payload:, headers:, verb:) + http_client.request( + path: path, + payload: payload, + headers: headers_with_default(headers), + verb: verb + ) + end + + def build_http_client(url, compress:) + uri = URI.parse(url) + raise "Invalid agentless mode URL: #{url}" if uri.host.nil? + + Datadog::CI::Transport::HTTP.new( + host: uri.host, + port: uri.port, + ssl: uri.scheme == "https" || uri.port == 443, + compress: compress + ) + end + def default_headers headers = super headers[Ext::Transport::HEADER_DD_API_KEY] = api_key diff --git a/lib/datadog/ci/transport/api/base.rb b/lib/datadog/ci/transport/api/base.rb index 9e9f053c..ff11e7e8 100644 --- a/lib/datadog/ci/transport/api/base.rb +++ b/lib/datadog/ci/transport/api/base.rb @@ -7,30 +7,23 @@ module CI module Transport module Api class Base - attr_reader :http + def api_request(path:, payload:, headers: {}, verb: "post") + headers[Ext::Transport::HEADER_CONTENT_TYPE] ||= Ext::Transport::CONTENT_TYPE_JSON + end - def initialize(http:) - @http = http + def citestcycle_request(path:, payload:, headers: {}, verb: "post") + headers[Ext::Transport::HEADER_CONTENT_TYPE] ||= Ext::Transport::CONTENT_TYPE_MESSAGEPACK end - def request(path:, payload:, headers: {}, verb: "post") + def headers_with_default(headers) request_headers = default_headers request_headers.merge!(headers) - - http.request( - path: path, - payload: payload, - verb: verb, - headers: request_headers - ) end private def default_headers - { - Ext::Transport::HEADER_CONTENT_TYPE => Ext::Transport::CONTENT_TYPE_MESSAGEPACK - } + {} end end end diff --git a/lib/datadog/ci/transport/api/builder.rb b/lib/datadog/ci/transport/api/builder.rb index 2e845deb..fddc3002 100644 --- a/lib/datadog/ci/transport/api/builder.rb +++ b/lib/datadog/ci/transport/api/builder.rb @@ -17,20 +17,14 @@ def self.build_agentless_api(settings) return nil if settings.api_key.nil? dd_site = settings.site || Ext::Transport::DEFAULT_DD_SITE - url = settings.ci.agentless_url || - "https://#{Ext::Transport::TEST_VISIBILITY_INTAKE_HOST_PREFIX}.#{dd_site}:443" - uri = URI.parse(url) - raise "Invalid agentless mode URL: #{url}" if uri.host.nil? + citestcycle_url = settings.ci.agentless_url || + "https://#{Ext::Transport::TEST_VISIBILITY_INTAKE_HOST_PREFIX}.#{dd_site}:443" - http = Datadog::CI::Transport::HTTP.new( - host: uri.host, - port: uri.port, - ssl: uri.scheme == "https" || uri.port == 443, - compress: true - ) + api_url = settings.ci.agentless_url || + "https://#{Ext::Transport::DD_API_HOST_PREFIX}.#{dd_site}:443" - Agentless.new(api_key: settings.api_key, http: http) + Agentless.new(api_key: settings.api_key, citestcycle_url: citestcycle_url, api_url: api_url) end def self.build_evp_proxy_api(settings) @@ -46,15 +40,7 @@ def self.build_evp_proxy_api(settings) return nil if evp_proxy_path_prefix.nil? - http = Datadog::CI::Transport::HTTP.new( - host: agent_settings.hostname, - port: agent_settings.port, - ssl: agent_settings.ssl, - timeout: agent_settings.timeout_seconds, - compress: Ext::Transport::EVP_PROXY_COMPRESSION_SUPPORTED[evp_proxy_path_prefix] - ) - - EvpProxy.new(http: http, path_prefix: evp_proxy_path_prefix) + EvpProxy.new(agent_settings: agent_settings, path_prefix: evp_proxy_path_prefix) end end end diff --git a/lib/datadog/ci/transport/api/evp_proxy.rb b/lib/datadog/ci/transport/api/evp_proxy.rb index 05d18675..8c93e32d 100644 --- a/lib/datadog/ci/transport/api/evp_proxy.rb +++ b/lib/datadog/ci/transport/api/evp_proxy.rb @@ -10,25 +10,48 @@ module CI module Transport module Api class EvpProxy < Base - def initialize(http:, path_prefix: Ext::Transport::EVP_PROXY_V2_PATH_PREFIX) - super(http: http) + def initialize(agent_settings:, path_prefix: Ext::Transport::EVP_PROXY_V2_PATH_PREFIX) + @agent_intake_http = build_http_client( + agent_settings, + compress: Ext::Transport::EVP_PROXY_COMPRESSION_SUPPORTED[path_prefix] + ) + + @agent_api_http = build_http_client(agent_settings, compress: false) path_prefix = "#{path_prefix}/" unless path_prefix.end_with?("/") @path_prefix = path_prefix end - def request(path:, payload:, headers: {}, verb: "post") - path = "#{@path_prefix}#{path.sub(/^\//, "")}" + def citestcycle_request(path:, payload:, headers: {}, verb: "post") + super + + headers[Ext::Transport::HEADER_EVP_SUBDOMAIN] = Ext::Transport::TEST_VISIBILITY_INTAKE_HOST_PREFIX + + perform_request(@agent_intake_http, path: path, payload: payload, headers: headers, verb: verb) + end + + def api_request(path:, payload:, headers: {}, verb: "post") + super + + headers[Ext::Transport::HEADER_EVP_SUBDOMAIN] = Ext::Transport::DD_API_HOST_PREFIX + + perform_request(@agent_api_http, path: path, payload: payload, headers: headers, verb: verb) + end + + private - super( - path: path, + def perform_request(http_client, path:, payload:, headers:, verb:) + http_client.request( + path: path_with_prefix(path), payload: payload, - headers: headers, + headers: headers_with_default(headers), verb: verb ) end - private + def path_with_prefix(path) + "#{@path_prefix}#{path.sub(/^\//, "")}" + end def container_id return @container_id if defined?(@container_id) @@ -38,13 +61,22 @@ def container_id def default_headers headers = super - headers[Ext::Transport::HEADER_EVP_SUBDOMAIN] = Ext::Transport::TEST_VISIBILITY_INTAKE_HOST_PREFIX c_id = container_id headers[Ext::Transport::HEADER_CONTAINER_ID] = c_id unless c_id.nil? headers end + + def build_http_client(agent_settings, compress:) + Datadog::CI::Transport::HTTP.new( + host: agent_settings.hostname, + port: agent_settings.port, + ssl: agent_settings.ssl, + timeout: agent_settings.timeout_seconds, + compress: compress + ) + end end end end diff --git a/sig/datadog/ci/ext/transport.rbs b/sig/datadog/ci/ext/transport.rbs index cfbd6511..75a9cd35 100644 --- a/sig/datadog/ci/ext/transport.rbs +++ b/sig/datadog/ci/ext/transport.rbs @@ -26,6 +26,12 @@ module Datadog TEST_VISIBILITY_INTAKE_PATH: "/api/v2/citestcycle" + DD_API_HOST_PREFIX: "api" + + DD_API_SETTINGS_PATH: "/api/v2/ci/libraries/tests/services/setting" + + DD_API_SETTINGS_TYPE: "ci_app_test_service_libraries_settings" + CONTENT_TYPE_MESSAGEPACK: "application/msgpack" CONTENT_TYPE_JSON: "application/json" diff --git a/sig/datadog/ci/transport/api/agentless.rbs b/sig/datadog/ci/transport/api/agentless.rbs index bbc7f206..04ae60da 100644 --- a/sig/datadog/ci/transport/api/agentless.rbs +++ b/sig/datadog/ci/transport/api/agentless.rbs @@ -6,13 +6,19 @@ module Datadog attr_reader api_key: String @api_key: String + @citestcycle_http: Datadog::CI::Transport::HTTP + @api_http: Datadog::CI::Transport::HTTP - def initialize: (api_key: String, http: Datadog::CI::Transport::HTTP) -> void + def initialize: (api_key: String, citestcycle_url: String, api_url: String) -> void def request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator private + def perform_request: (Datadog::CI::Transport::HTTP client, path: String, payload: String, headers: Hash[String, String], verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator + + def build_http_client: (String url, compress: bool) -> Datadog::CI::Transport::HTTP + def default_headers: () -> Hash[String, String] end end diff --git a/sig/datadog/ci/transport/api/base.rbs b/sig/datadog/ci/transport/api/base.rbs index c2f5b458..21cc7f49 100644 --- a/sig/datadog/ci/transport/api/base.rbs +++ b/sig/datadog/ci/transport/api/base.rbs @@ -3,16 +3,14 @@ module Datadog module Transport module Api class Base - attr_reader http: Datadog::CI::Transport::HTTP + def api_request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> untyped - @http: Datadog::CI::Transport::HTTP - - def initialize: (http: Datadog::CI::Transport::HTTP) -> void - - def request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> untyped + def citestcycle_request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> untyped private + def headers_with_default: (Hash[String, String] headers) -> Hash[String, String] + def default_headers: () -> Hash[String, String] end end diff --git a/sig/datadog/ci/transport/api/evp_proxy.rbs b/sig/datadog/ci/transport/api/evp_proxy.rbs index 34e064ff..56d68f38 100644 --- a/sig/datadog/ci/transport/api/evp_proxy.rbs +++ b/sig/datadog/ci/transport/api/evp_proxy.rbs @@ -3,15 +3,27 @@ module Datadog module Transport module Api class EvpProxy < Base + @agent_intake_http: Datadog::CI::Transport::HTTP + @agent_api_http: Datadog::CI::Transport::HTTP @container_id: String? @path_prefix: String - def initialize: (http: Datadog::CI::Transport::HTTP, ?path_prefix: String) -> void + def initialize: (agent_settings: Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, ?path_prefix: String) -> void def request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator + def citestcycle_request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator + + def api_request: (path: String, payload: String, ?headers: Hash[String, String], ?verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator + private + def perform_request: (Datadog::CI::Transport::HTTP client, path: String, payload: String, headers: Hash[String, String], verb: ::String) -> Datadog::CI::Transport::HTTP::ResponseDecorator + + def build_http_client: (Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings, compress: bool) -> Datadog::CI::Transport::HTTP + + def path_with_prefix: (String path) -> String + def container_id: () -> String? def default_headers: () -> Hash[String, String] diff --git a/spec/datadog/ci/test_visibility/transport_spec.rb b/spec/datadog/ci/test_visibility/transport_spec.rb index ebf0f2f0..e3fe6b5c 100644 --- a/spec/datadog/ci/test_visibility/transport_spec.rb +++ b/spec/datadog/ci/test_visibility/transport_spec.rb @@ -33,7 +33,7 @@ it "sends correct payload" do subject.send_traces([trace]) - expect(api).to have_received(:request) do |args| + expect(api).to have_received(:citestcycle_request) do |args| expect(args[:path]).to eq("/api/v2/citestcycle") payload = MessagePack.unpack(args[:payload]) @@ -59,7 +59,7 @@ it "sends correct payload including env" do subject.send_traces([trace]) - expect(api).to have_received(:request) do |args| + expect(api).to have_received(:citestcycle_request) do |args| payload = MessagePack.unpack(args[:payload]) metadata = payload["metadata"]["*"] @@ -79,7 +79,7 @@ it "sends event for each of spans" do subject.send_traces(traces) - expect(api).to have_received(:request) do |args| + expect(api).to have_received(:citestcycle_request) do |args| payload = MessagePack.unpack(args[:payload]) events = payload["events"] expect(events.count).to eq(expected_events_count) @@ -97,7 +97,7 @@ it "filters out invalid events" do subject.send_traces(traces) - expect(api).to have_received(:request) do |args| + expect(api).to have_received(:citestcycle_request) do |args| payload = MessagePack.unpack(args[:payload]) events = payload["events"] @@ -127,7 +127,7 @@ it "filters out invalid events" do responses = subject.send_traces(traces) - expect(api).to have_received(:request).twice + expect(api).to have_received(:citestcycle_request).twice expect(responses.count).to eq(2) end end @@ -140,7 +140,7 @@ it "does not send events that are larger than max size" do subject.send_traces(traces) - expect(api).not_to have_received(:request) + expect(api).not_to have_received(:citestcycle_request) end end end @@ -155,7 +155,7 @@ it "does not send anything" do subject.send_traces(traces) - expect(api).not_to have_received(:request) + expect(api).not_to have_received(:citestcycle_request) end end end diff --git a/spec/datadog/ci/transport/api/agentless_spec.rb b/spec/datadog/ci/transport/api/agentless_spec.rb index 7f11848c..77ca6884 100644 --- a/spec/datadog/ci/transport/api/agentless_spec.rb +++ b/spec/datadog/ci/transport/api/agentless_spec.rb @@ -4,44 +4,139 @@ subject do described_class.new( api_key: api_key, - http: http + citestcycle_url: citestcycle_url, + api_url: api_url ) end let(:api_key) { "api_key" } - let(:http) { double(:http) } - describe "#request" do + context "malformed urls" do + let(:citestcycle_url) { "" } + let(:api_url) { "api.datadoghq.com" } + + it { expect { subject }.to raise_error(/Invalid agentless mode URL:/) } + end + + context "http urls" do + let(:citestcycle_url) { "http://localhost:5555" } + let(:citestcycle_http) { double(:http) } + + let(:api_url) { "http://localhost:5555" } + let(:api_http) { double(:http) } + before do - allow(Datadog::CI::Transport::HTTP).to receive(:new).and_return(http) + expect(Datadog::CI::Transport::HTTP).to receive(:new).with( + host: "localhost", + port: 5555, + ssl: false, + compress: true + ).and_return(citestcycle_http) + + expect(Datadog::CI::Transport::HTTP).to receive(:new).with( + host: "localhost", + port: 5555, + ssl: false, + compress: false + ).and_return(api_http) end - it "produces correct headers and forwards request to HTTP layer" do - expect(http).to receive(:request).with( - path: "path", - payload: "payload", - verb: "post", - headers: { + describe "#citestcycle_request" do + let(:expected_headers) do + { "DD-API-KEY" => "api_key", "Content-Type" => "application/msgpack" } - ) + end + + it "produces correct headers and forwards request to HTTP layer" do + expect(citestcycle_http).to receive(:request).with( + path: "path", + payload: "payload", + verb: "post", + headers: expected_headers + ) - subject.request(path: "path", payload: "payload") + subject.citestcycle_request(path: "path", payload: "payload") + end end + end + + context "valid urls" do + let(:citestcycle_url) { "https://citestcycle-intake.datadoghq.com:443" } + let(:citestcycle_http) { double(:http) } + + let(:api_url) { "https://api.datadoghq.com:443" } + let(:api_http) { double(:http) } - it "alows to override headers" do - expect(http).to receive(:request).with( - path: "path", - payload: "payload", - verb: "post", - headers: { + before do + expect(Datadog::CI::Transport::HTTP).to receive(:new).with( + host: "citestcycle-intake.datadoghq.com", + port: 443, + ssl: true, + compress: true + ).and_return(citestcycle_http) + + expect(Datadog::CI::Transport::HTTP).to receive(:new).with( + host: "api.datadoghq.com", + port: 443, + ssl: true, + compress: false + ).and_return(api_http) + end + + describe "#citestcycle_request" do + let(:expected_headers) do + { + "DD-API-KEY" => "api_key", + "Content-Type" => "application/msgpack" + } + end + + it "produces correct headers and forwards request to HTTP layer" do + expect(citestcycle_http).to receive(:request).with( + path: "path", + payload: "payload", + verb: "post", + headers: expected_headers + ) + + subject.citestcycle_request(path: "path", payload: "payload") + end + + it "alows to override headers" do + expect(citestcycle_http).to receive(:request).with( + path: "path", + payload: "payload", + verb: "post", + headers: expected_headers.merge({"Content-Type" => "application/json"}) + ) + + subject.citestcycle_request(path: "path", payload: "payload", headers: {"Content-Type" => "application/json"}) + end + end + + describe "#api_request" do + let(:expected_headers) do + { "DD-API-KEY" => "api_key", - "Content-Type" => "application/json" + "Content-Type" => "application/msgpack" } - ) + end + + it "produces correct headers and forwards request to HTTP layer" do + expect(api_http).to receive(:request).with( + path: "path", + payload: "payload", + verb: "post", + headers: { + "DD-API-KEY" => "api_key", + "Content-Type" => "application/json" + } + ) - subject.request(path: "path", payload: "payload", headers: {"Content-Type" => "application/json"}) + subject.api_request(path: "path", payload: "payload") + end end end end diff --git a/spec/datadog/ci/transport/api/builder_spec.rb b/spec/datadog/ci/transport/api/builder_spec.rb index 8fdb6c98..e966107b 100644 --- a/spec/datadog/ci/transport/api/builder_spec.rb +++ b/spec/datadog/ci/transport/api/builder_spec.rb @@ -16,7 +16,6 @@ subject { described_class.build_agentless_api(settings) } let(:api) { double(:api) } - let(:http) { double(:http) } let(:agentless_url) { nil } let(:dd_site) { nil } let(:api_key) { "api_key" } @@ -37,15 +36,8 @@ end it "creates and configures http client and Agentless api" do - expect(Datadog::CI::Transport::HTTP).to receive(:new).with( - host: "citestcycle-intake.datadoghq.com", - port: 443, - ssl: true, - compress: true - ).and_return(http) - expect(Datadog::CI::Transport::Api::Agentless).to receive(:new).with( - api_key: "api_key", http: http + api_key: "api_key", citestcycle_url: "https://citestcycle-intake.datadoghq.com:443", api_url: "https://api.datadoghq.com:443" ).and_return(api) expect(subject).to eq(api) @@ -55,15 +47,8 @@ let(:agentless_url) { "http://localhost:5555" } it "configures transport to use intake URL from settings" do - expect(Datadog::CI::Transport::HTTP).to receive(:new).with( - host: "localhost", - port: 5555, - ssl: false, - compress: true - ).and_return(http) - expect(Datadog::CI::Transport::Api::Agentless).to receive(:new).with( - api_key: "api_key", http: http + api_key: "api_key", citestcycle_url: "http://localhost:5555", api_url: "http://localhost:5555" ).and_return(api) expect(subject).to eq(api) @@ -74,15 +59,8 @@ let(:dd_site) { "datadoghq.eu" } it "construct intake url using provided host" do - expect(Datadog::CI::Transport::HTTP).to receive(:new).with( - host: "citestcycle-intake.datadoghq.eu", - port: 443, - ssl: true, - compress: true - ).and_return(http) - expect(Datadog::CI::Transport::Api::Agentless).to receive(:new).with( - api_key: "api_key", http: http + api_key: "api_key", citestcycle_url: "https://citestcycle-intake.datadoghq.eu:443", api_url: "https://api.datadoghq.eu:443" ).and_return(api) expect(subject).to eq(api) @@ -94,7 +72,6 @@ subject { described_class.build_evp_proxy_api(settings) } let(:api) { double(:api) } - let(:http) { double(:http) } let(:agent_settings) do Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( @@ -133,16 +110,8 @@ end it "creates and configures http client and EvpProxy" do - expect(Datadog::CI::Transport::HTTP).to receive(:new).with( - host: "localhost", - port: 5555, - ssl: false, - timeout: 42, - compress: false - ).and_return(http) - expect(Datadog::CI::Transport::Api::EvpProxy).to( - receive(:new).with(http: http, path_prefix: "/evp_proxy/v2/").and_return(api) + receive(:new).with(agent_settings: agent_settings, path_prefix: "/evp_proxy/v2/").and_return(api) ) expect(subject).to eq(api) @@ -157,16 +126,8 @@ end it "creates and configures http client and EvpProxy" do - expect(Datadog::CI::Transport::HTTP).to receive(:new).with( - host: "localhost", - port: 5555, - ssl: false, - timeout: 42, - compress: true - ).and_return(http) - expect(Datadog::CI::Transport::Api::EvpProxy).to( - receive(:new).with(http: http, path_prefix: "/evp_proxy/v4/").and_return(api) + receive(:new).with(agent_settings: agent_settings, path_prefix: "/evp_proxy/v4/").and_return(api) ) expect(subject).to eq(api) diff --git a/spec/datadog/ci/transport/api/evp_proxy_spec.rb b/spec/datadog/ci/transport/api/evp_proxy_spec.rb index e7b7d531..50a1c521 100644 --- a/spec/datadog/ci/transport/api/evp_proxy_spec.rb +++ b/spec/datadog/ci/transport/api/evp_proxy_spec.rb @@ -2,102 +2,199 @@ RSpec.describe Datadog::CI::Transport::Api::EvpProxy do subject do - described_class.new(http: http, path_prefix: path_prefix) + described_class.new(agent_settings: agent_settings, path_prefix: path_prefix) end - let(:http) { double(:http) } - let(:path_prefix) { "/evp_proxy/v2/" } + let(:agent_settings) do + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( + adapter: :net_http, + ssl: false, + hostname: "localhost", + port: 5555, + uds_path: nil, + timeout_seconds: 42, + deprecated_for_removal_transport_configuration_proc: nil + ) + end + let(:intake_http) { double(:http) } + let(:api_http) { double(:http) } + + let(:container_id) { nil } + before do + expect(Datadog::Core::Environment::Container).to receive(:container_id).and_return(container_id) + end + + let(:citestcycle_headers) do + { + "Content-Type" => "application/msgpack", + "X-Datadog-EVP-Subdomain" => "citestcycle-intake" + } + end + + let(:api_headers) do + { + "Content-Type" => "application/json", + "X-Datadog-EVP-Subdomain" => "api" + } + end - describe "#request" do - let(:container_id) { nil } + context "with evp proxy v2" do + let(:path_prefix) { Datadog::CI::Ext::Transport::EVP_PROXY_V2_PATH_PREFIX } before do - allow(Datadog::CI::Transport::HTTP).to receive(:new).and_return(http) - expect(Datadog::Core::Environment::Container).to receive(:container_id).and_return(container_id) + expect(Datadog::CI::Transport::HTTP).to receive(:new).with( + host: agent_settings.hostname, + port: agent_settings.port, + ssl: agent_settings.ssl, + timeout: agent_settings.timeout_seconds, + compress: false + ).and_return(intake_http, api_http) end - context "with path starting from / character" do - it "produces correct headers and forwards request to HTTP layer prepending path with evp_proxy" do - expect(http).to receive(:request).with( - path: "/evp_proxy/v2/path", - payload: "payload", - verb: "post", - headers: { - "Content-Type" => "application/msgpack", - "X-Datadog-EVP-Subdomain" => "citestcycle-intake" - } - ) + describe "#citestcycle_request" do + context "with path starting from / character" do + it "produces correct headers and forwards request to HTTP layer prepending path with evp_proxy" do + expect(intake_http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: citestcycle_headers + ) + + subject.citestcycle_request(path: "/path", payload: "payload") + end + end - subject.request(path: "/path", payload: "payload") + context "with path without / in the beginning" do + it "constructs evp proxy path correctly" do + expect(intake_http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: citestcycle_headers + ) + + subject.citestcycle_request(path: "path", payload: "payload") + end end - end - context "with path without / in the beginning" do - it "constructs evp proxy path correctly" do - expect(http).to receive(:request).with( - path: "/evp_proxy/v2/path", - payload: "payload", - verb: "post", - headers: { - "Content-Type" => "application/msgpack", - "X-Datadog-EVP-Subdomain" => "citestcycle-intake" - } - ) + context "with container id" do + let(:container_id) { "container-id" } + + it "adds an additional Datadog-Container-ID header" do + expect(intake_http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: citestcycle_headers.merge("Datadog-Container-ID" => "container-id") + ) - subject.request(path: "path", payload: "payload") + subject.citestcycle_request(path: "/path", payload: "payload") + end + end + + context "overriding content-type" do + it "uses content type header from the request parameter" do + expect(intake_http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: citestcycle_headers.merge({"Content-Type" => "application/json"}) + ) + + subject.citestcycle_request(path: "/path", payload: "payload", headers: {"Content-Type" => "application/json"}) + end end end - context "with different path_prefix" do - let(:path_prefix) { "/evp_proxy/v4" } + describe "#api_request" do + context "with path starting from / character" do + it "produces correct headers and forwards request to HTTP layer prepending path with evp_proxy" do + expect(api_http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: api_headers + ) + + subject.api_request(path: "/path", payload: "payload") + end + end - it "constructs evp proxy path using this prefix" do - expect(http).to receive(:request).with( - path: "/evp_proxy/v4/path", - payload: "payload", - verb: "post", - headers: { - "Content-Type" => "application/msgpack", - "X-Datadog-EVP-Subdomain" => "citestcycle-intake" - } - ) + context "with path without / in the beginning" do + it "constructs evp proxy path correctly" do + expect(api_http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: api_headers + ) + + subject.api_request(path: "path", payload: "payload") + end + end - subject.request(path: "path", payload: "payload") + context "with container id" do + let(:container_id) { "container-id" } + + it "adds an additional Datadog-Container-ID header" do + expect(api_http).to receive(:request).with( + path: "/evp_proxy/v2/path", + payload: "payload", + verb: "post", + headers: api_headers.merge("Datadog-Container-ID" => "container-id") + ) + + subject.api_request(path: "/path", payload: "payload") + end end end + end + + context "with evp proxy v4" do + let(:path_prefix) { Datadog::CI::Ext::Transport::EVP_PROXY_V4_PATH_PREFIX } - context "with container id" do - let(:container_id) { "container-id" } + before do + expect(Datadog::CI::Transport::HTTP).to receive(:new).with( + host: agent_settings.hostname, + port: agent_settings.port, + ssl: agent_settings.ssl, + timeout: agent_settings.timeout_seconds, + compress: true + ).and_return(intake_http) + + expect(Datadog::CI::Transport::HTTP).to receive(:new).with( + host: agent_settings.hostname, + port: agent_settings.port, + ssl: agent_settings.ssl, + timeout: agent_settings.timeout_seconds, + compress: false + ).and_return(api_http) + end - it "adds an additional Datadog-Container-ID header" do - expect(http).to receive(:request).with( - path: "/evp_proxy/v2/path", + describe "#citestcycle_request" do + it "constructs evp proxy path using v4 prefix" do + expect(intake_http).to receive(:request).with( + path: "/evp_proxy/v4/path", payload: "payload", verb: "post", - headers: { - "Content-Type" => "application/msgpack", - "X-Datadog-EVP-Subdomain" => "citestcycle-intake", - "Datadog-Container-ID" => "container-id" - } + headers: citestcycle_headers ) - subject.request(path: "/path", payload: "payload") + subject.citestcycle_request(path: "path", payload: "payload") end end - context "overriding content-type" do - it "uses content type header from the request parameter" do - expect(http).to receive(:request).with( - path: "/evp_proxy/v2/path", + describe "#api_request" do + it "constructs evp proxy path using v4 prefix" do + expect(api_http).to receive(:request).with( + path: "/evp_proxy/v4/path", payload: "payload", verb: "post", - headers: { - "Content-Type" => "application/json", - "X-Datadog-EVP-Subdomain" => "citestcycle-intake" - } + headers: api_headers ) - subject.request(path: "/path", payload: "payload", headers: {"Content-Type" => "application/json"}) + subject.api_request(path: "path", payload: "payload") end end end From 286ed0cb81fd7bb77b3c058558826e549b5da15a Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Mon, 4 Mar 2024 16:01:18 +0100 Subject: [PATCH 07/20] decouple API client that requests library settings from ITR --- lib/datadog/ci/configuration/components.rb | 8 +++-- lib/datadog/ci/ext/itr.rb | 13 ------- lib/datadog/ci/itr/runner.rb | 16 ++------- lib/datadog/ci/test_visibility/recorder.rb | 26 +++++++++----- .../client.rb => transport/api_client.rb} | 26 +++++++------- sig/datadog/ci/ext/itr.rbs | 11 ------ sig/datadog/ci/itr/runner.rbs | 5 ++- sig/datadog/ci/test_visibility/recorder.rbs | 5 ++- .../client.rbs => transport/api_client.rbs} | 6 ++-- spec/datadog/ci/itr/client_spec.rb | 33 ------------------ spec/datadog/ci/itr/runner_spec.rb | 34 +++---------------- .../ci/test_visibility/recorder_spec.rb | 10 ------ spec/datadog/ci/transport/api_client_spec.rb | 31 +++++++++++++++++ 13 files changed, 81 insertions(+), 143 deletions(-) delete mode 100644 lib/datadog/ci/ext/itr.rb rename lib/datadog/ci/{itr/client.rb => transport/api_client.rb} (51%) delete mode 100644 sig/datadog/ci/ext/itr.rbs rename sig/datadog/ci/{itr/client.rbs => transport/api_client.rbs} (67%) delete mode 100644 spec/datadog/ci/itr/client_spec.rb create mode 100644 spec/datadog/ci/transport/api_client_spec.rb diff --git a/lib/datadog/ci/configuration/components.rb b/lib/datadog/ci/configuration/components.rb index 4c111d0d..23fc4309 100644 --- a/lib/datadog/ci/configuration/components.rb +++ b/lib/datadog/ci/configuration/components.rb @@ -71,14 +71,16 @@ def activate_ci!(settings) settings.tracing.test_mode.writer_options = writer_options itr = Datadog::CI::ITR::Runner.new( - enabled: settings.ci.enabled && settings.ci.itr_enabled, - api: test_visibility_api + enabled: settings.ci.enabled && settings.ci.itr_enabled ) + api_client = Transport::ApiClient.new(api: test_visibility_api) + # CI visibility recorder global instance @ci_recorder = TestVisibility::Recorder.new( test_suite_level_visibility_enabled: !settings.ci.force_test_level_visibility, - itr: itr + itr: itr, + api_client: api_client ) end diff --git a/lib/datadog/ci/ext/itr.rb b/lib/datadog/ci/ext/itr.rb deleted file mode 100644 index 48ed8485..00000000 --- a/lib/datadog/ci/ext/itr.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module Datadog - module CI - module Ext - module ITR - API_TYPE_SETTINGS = "ci_app_test_service_libraries_settings" - - API_PATH_SETTINGS = "/api/v2/ci/libraries/tests/services/setting" - end - end - end -end diff --git a/lib/datadog/ci/itr/runner.rb b/lib/datadog/ci/itr/runner.rb index 651ed8c2..00b46537 100644 --- a/lib/datadog/ci/itr/runner.rb +++ b/lib/datadog/ci/itr/runner.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative "client" - module Datadog module CI module ITR @@ -10,25 +8,17 @@ module ITR # skip tests that are not impacted by the changes class Runner def initialize( - enabled: false, - api: nil + enabled: false ) @enabled = enabled - return unless enabled - - @client = Client.new(api: api) end def enabled? @enabled end - def configure(service:) - return unless enabled? - - # TODO: error handling when request failed - # TODO: need to pass runtime information - @client.fetch_settings(service: service) + def disable + @enabled = false end end end diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index 7b45594c..bf01e9a2 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -12,6 +12,7 @@ require_relative "../ext/app_types" require_relative "../ext/test" require_relative "../ext/environment" +require_relative "../transport/api_client" require_relative "../utils/git" require_relative "../span" @@ -29,9 +30,8 @@ class Recorder attr_reader :environment_tags, :test_suite_level_visibility_enabled def initialize( - test_suite_level_visibility_enabled: false, - codeowners: Codeowners::Parser.new(Utils::Git.root).parse, - itr: nil + itr:, api_client:, test_suite_level_visibility_enabled: false, + codeowners: Codeowners::Parser.new(Utils::Git.root).parse ) @test_suite_level_visibility_enabled = test_suite_level_visibility_enabled @@ -41,24 +41,24 @@ def initialize( @codeowners = codeowners - raise ArgumentError, "ITR runner is required" unless itr @itr = itr + @api_client = api_client end def start_test_session(service: nil, tags: {}) return skip_tracing unless test_suite_level_visibility_enabled - # TODO: pass runtime and environment information - # actually this is the correct place to configure ITR - when starting test session - @itr.configure(service: service) - @global_context.fetch_or_activate_test_session do tracer_span = start_datadog_tracer_span( "test.session", build_span_options(service, Ext::AppTypes::TYPE_TEST_SESSION) ) set_session_context(tags, tracer_span) - build_test_session(tracer_span, tags) + test_session = build_test_session(tracer_span, tags) + + configure_library(service: service) + + test_session end end @@ -190,6 +190,14 @@ def itr_enabled? private + def configure_library(service:) + return unless itr_enabled? + # TODO: error handling when request failed - disable ITR runner + # TODO: need to pass runtime information + # TODO: disable ITR runner if itr_enabled is false in settings + @api_client.fetch_library_settings(service: service) + end + def skip_tracing(block = nil) block.call(nil) if block end diff --git a/lib/datadog/ci/itr/client.rb b/lib/datadog/ci/transport/api_client.rb similarity index 51% rename from lib/datadog/ci/itr/client.rb rename to lib/datadog/ci/transport/api_client.rb index f8f19693..d81da6cc 100644 --- a/lib/datadog/ci/itr/client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -1,28 +1,26 @@ # frozen_string_literal: true -require_relative "../ext/itr" +require_relative "../ext/transport" module Datadog module CI - module ITR - # ITR API client - # communicates with the CI visibility backend - class Client + module Transport + # Datadog API client + # Calls settings endpoint to fetch library settings for given service and env + class ApiClient def initialize(api: nil) - raise ArgumentError, "Test visibility API is required" unless api - @api = api end - def fetch_settings(service:) - # TODO: application/json support + def fetch_library_settings(service:) + # TODO: return error response if api is not present + return {} unless @api # TODO: id generation # TODO: runtime information is required for payload # TODO: return error response - use some wrapper from ddtrace as an example - @api.request( - path: Ext::ITR::API_PATH_SETTINGS, - payload: settings_payload(service: service), - headers: {Ext::Transport::HEADER_CONTENT_TYPE => Ext::Transport::CONTENT_TYPE_JSON} + @api.api_request( + path: Ext::Transport::DD_API_SETTINGS_PATH, + payload: settings_payload(service: service) ) end @@ -32,7 +30,7 @@ def settings_payload(service:) { data: { id: "change_me", - type: Ext::ITR::API_TYPE_SETTINGS, + type: Ext::Transport::DD_API_SETTINGS_TYPE, attributes: { service: service } diff --git a/sig/datadog/ci/ext/itr.rbs b/sig/datadog/ci/ext/itr.rbs deleted file mode 100644 index 9296de59..00000000 --- a/sig/datadog/ci/ext/itr.rbs +++ /dev/null @@ -1,11 +0,0 @@ -module Datadog - module CI - module Ext - module ITR - API_TYPE_SETTINGS: "ci_app_test_service_libraries_settings" - - API_PATH_SETTINGS: "/api/v2/ci/libraries/tests/services/setting" - end - end - end -end diff --git a/sig/datadog/ci/itr/runner.rbs b/sig/datadog/ci/itr/runner.rbs index 5155a00a..37293e6c 100644 --- a/sig/datadog/ci/itr/runner.rbs +++ b/sig/datadog/ci/itr/runner.rbs @@ -3,13 +3,12 @@ module Datadog module ITR class Runner @enabled: bool - @client: Datadog::CI::ITR::Client - def initialize: (?enabled: bool, api: Datadog::CI::Transport::Api::Base?) -> void + def initialize: (?enabled: bool) -> void def enabled?: () -> bool - def configure: (service: String?) -> void + def disable: () -> void end end end diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index 130bbb37..a3e3ae94 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -8,12 +8,13 @@ module Datadog @local_context: Datadog::CI::TestVisibility::Context::Local @global_context: Datadog::CI::TestVisibility::Context::Global @itr: Datadog::CI::ITR::Runner + @api_client: Datadog::CI::Transport::ApiClient @codeowners: Datadog::CI::Codeowners::Matcher attr_reader environment_tags: Hash[String, String] attr_reader test_suite_level_visibility_enabled: bool - def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher, ?itr: Datadog::CI::ITR::Runner?) -> void + def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher, itr: Datadog::CI::ITR::Runner, api_client: Datadog::CI::Transport::ApiClient) -> void def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Test span) -> untyped } -> untyped @@ -47,6 +48,8 @@ module Datadog private + def configure_library: (service: String?) -> void + 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 diff --git a/sig/datadog/ci/itr/client.rbs b/sig/datadog/ci/transport/api_client.rbs similarity index 67% rename from sig/datadog/ci/itr/client.rbs rename to sig/datadog/ci/transport/api_client.rbs index 311ca3ef..463751f4 100644 --- a/sig/datadog/ci/itr/client.rbs +++ b/sig/datadog/ci/transport/api_client.rbs @@ -1,12 +1,12 @@ module Datadog module CI - module ITR - class Client + module Transport + class ApiClient @api: untyped def initialize: (?api: Datadog::CI::Transport::Api::Base?) -> void - def fetch_settings: (service: String?) -> untyped + def fetch_library_settings: (service: String?) -> untyped private diff --git a/spec/datadog/ci/itr/client_spec.rb b/spec/datadog/ci/itr/client_spec.rb deleted file mode 100644 index 9ab3cdb5..00000000 --- a/spec/datadog/ci/itr/client_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -require_relative "../../../../lib/datadog/ci/itr/client" -require_relative "../../../../lib/datadog/ci/ext/itr" - -RSpec.describe Datadog::CI::ITR::Client do - let(:api) { spy("api") } - subject { described_class.new(api: api) } - - describe "#fetch_settings" do - let(:service) { "service" } - let(:path) { Datadog::CI::Ext::ITR::API_PATH_SETTINGS } - let(:payload) do - { - data: { - id: "change_me", - type: Datadog::CI::Ext::ITR::API_TYPE_SETTINGS, - attributes: { - service: service - } - } - }.to_json - end - - it "requests the settings" do - subject.fetch_settings(service: service) - - expect(api).to have_received(:request).with( - path: path, - payload: payload, - headers: {"Content-Type" => "application/json"} - ) - end - end -end diff --git a/spec/datadog/ci/itr/runner_spec.rb b/spec/datadog/ci/itr/runner_spec.rb index 77b88447..fa6c90d0 100644 --- a/spec/datadog/ci/itr/runner_spec.rb +++ b/spec/datadog/ci/itr/runner_spec.rb @@ -1,41 +1,15 @@ # frozen_string_literal: true -require_relative "../../../../lib/datadog/ci/itr/client" require_relative "../../../../lib/datadog/ci/itr/runner" RSpec.describe Datadog::CI::ITR::Runner do let(:itr_enabled) { true } - let(:api) { double("api") } - let(:client) { double("client") } + subject(:runner) { described_class.new(enabled: itr_enabled) } - subject(:runner) { described_class.new(enabled: itr_enabled, api: api) } - - describe "#configure" do - let(:service) { "service" } - - context "itr enabled" do - before do - expect(Datadog::CI::ITR::Client).to receive(:new).with(api: api).and_return(client) - end - - it "fetches settings from backend" do - expect(client).to receive(:fetch_settings).with(service: service) - - runner.configure(service: service) - end - end - - context "itr disabled" do - let(:itr_enabled) { false } - - before do - expect(Datadog::CI::ITR::Client).to_not receive(:new) - end - - it "does nothing" do - expect(runner.configure(service: service)).to be_nil - end + describe "#disable" do + it "disables the runner" do + expect { runner.disable }.to change { runner.enabled? }.from(true).to(false) end end end diff --git a/spec/datadog/ci/test_visibility/recorder_spec.rb b/spec/datadog/ci/test_visibility/recorder_spec.rb index d8a0c1b6..4f074867 100644 --- a/spec/datadog/ci/test_visibility/recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/recorder_spec.rb @@ -48,16 +48,6 @@ end end - describe "#initialize" do - context "no ITR runner is provided" do - subject { described_class.new } - - it "raises an error" do - expect { subject }.to raise_error(ArgumentError, "ITR runner is required") - end - end - end - context "when test suite level visibility is disabled" do let(:service) { "my-service" } let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/api_client_spec.rb new file mode 100644 index 00000000..bb59ac9d --- /dev/null +++ b/spec/datadog/ci/transport/api_client_spec.rb @@ -0,0 +1,31 @@ +require_relative "../../../../lib/datadog/ci/transport/api_client" + +RSpec.describe Datadog::CI::Transport::ApiClient do + let(:api) { spy("api") } + subject { described_class.new(api: api) } + + describe "#fetch_library_settings" do + let(:service) { "service" } + let(:path) { Datadog::CI::Ext::Transport::DD_API_SETTINGS_PATH } + let(:payload) do + { + data: { + id: "change_me", + type: Datadog::CI::Ext::Transport::DD_API_SETTINGS_TYPE, + attributes: { + service: service + } + } + }.to_json + end + + it "requests the settings" do + subject.fetch_library_settings(service: service) + + expect(api).to have_received(:api_request).with( + path: path, + payload: payload + ) + end + end +end From b5c0aff376396986ab3c054f89a438ab7083becc Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Mon, 4 Mar 2024 16:05:39 +0100 Subject: [PATCH 08/20] send correct runtime id in the library settings request --- lib/datadog/ci/transport/api_client.rb | 4 +++- spec/datadog/ci/transport/api_client_spec.rb | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/api_client.rb index d81da6cc..9382ab07 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "datadog/core/environment/identity" + require_relative "../ext/transport" module Datadog @@ -29,7 +31,7 @@ def fetch_library_settings(service:) def settings_payload(service:) { data: { - id: "change_me", + id: Datadog::Core::Environment::Identity.id, type: Ext::Transport::DD_API_SETTINGS_TYPE, attributes: { service: service diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/api_client_spec.rb index bb59ac9d..f5c860c1 100644 --- a/spec/datadog/ci/transport/api_client_spec.rb +++ b/spec/datadog/ci/transport/api_client_spec.rb @@ -22,10 +22,17 @@ it "requests the settings" do subject.fetch_library_settings(service: service) - expect(api).to have_received(:api_request).with( - path: path, - payload: payload - ) + expect(api).to have_received(:api_request) do |args| + expect(args[:path]).to eq(path) + + data = JSON.parse(args[:payload])["data"] + + expect(data["id"]).to eq(Datadog::Core::Environment::Identity.id) + expect(data["type"]).to eq(Datadog::CI::Ext::Transport::DD_API_SETTINGS_TYPE) + + attributes = data["attributes"] + expect(attributes["service"]).to eq(service) + end end end end From c99bd958e8743395f9c9fea45d2e4f5f1eb146b2 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Mon, 4 Mar 2024 16:20:37 +0100 Subject: [PATCH 09/20] add DD_ENV to library settings request --- lib/datadog/ci/configuration/components.rb | 5 ++++- lib/datadog/ci/transport/api_client.rb | 6 ++++-- sig/datadog/ci/transport/api_client.rbs | 5 +++-- spec/datadog/ci/transport/api_client_spec.rb | 5 ++++- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/datadog/ci/configuration/components.rb b/lib/datadog/ci/configuration/components.rb index 23fc4309..345f256d 100644 --- a/lib/datadog/ci/configuration/components.rb +++ b/lib/datadog/ci/configuration/components.rb @@ -74,7 +74,10 @@ def activate_ci!(settings) enabled: settings.ci.enabled && settings.ci.itr_enabled ) - api_client = Transport::ApiClient.new(api: test_visibility_api) + api_client = Transport::ApiClient.new( + api: test_visibility_api, + dd_env: settings.env + ) # CI visibility recorder global instance @ci_recorder = TestVisibility::Recorder.new( diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/api_client.rb index 9382ab07..49115148 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -10,8 +10,9 @@ module Transport # Datadog API client # Calls settings endpoint to fetch library settings for given service and env class ApiClient - def initialize(api: nil) + def initialize(api: nil, dd_env: nil) @api = api + @dd_env = dd_env end def fetch_library_settings(service:) @@ -34,7 +35,8 @@ def settings_payload(service:) id: Datadog::Core::Environment::Identity.id, type: Ext::Transport::DD_API_SETTINGS_TYPE, attributes: { - service: service + service: service, + env: @dd_env } } }.to_json diff --git a/sig/datadog/ci/transport/api_client.rbs b/sig/datadog/ci/transport/api_client.rbs index 463751f4..06d3d7ab 100644 --- a/sig/datadog/ci/transport/api_client.rbs +++ b/sig/datadog/ci/transport/api_client.rbs @@ -2,9 +2,10 @@ module Datadog module CI module Transport class ApiClient - @api: untyped + @api: Datadog::CI::Transport::Api::Base? + @dd_env: String? - def initialize: (?api: Datadog::CI::Transport::Api::Base?) -> void + def initialize: (?api: Datadog::CI::Transport::Api::Base?, ?dd_env: String?) -> void def fetch_library_settings: (service: String?) -> untyped diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/api_client_spec.rb index f5c860c1..94997db7 100644 --- a/spec/datadog/ci/transport/api_client_spec.rb +++ b/spec/datadog/ci/transport/api_client_spec.rb @@ -2,7 +2,9 @@ RSpec.describe Datadog::CI::Transport::ApiClient do let(:api) { spy("api") } - subject { described_class.new(api: api) } + let(:dd_env) { "ci" } + + subject { described_class.new(api: api, dd_env: dd_env) } describe "#fetch_library_settings" do let(:service) { "service" } @@ -32,6 +34,7 @@ attributes = data["attributes"] expect(attributes["service"]).to eq(service) + expect(attributes["env"]).to eq(dd_env) end end end From e67b976e417ec54cdc1a938b60a4d2468341387b Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Mon, 4 Mar 2024 16:23:52 +0100 Subject: [PATCH 10/20] typechecking fix --- lib/datadog/ci/transport/api_client.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/api_client.rb index 49115148..30ee2ec6 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -17,11 +17,12 @@ def initialize(api: nil, dd_env: nil) def fetch_library_settings(service:) # TODO: return error response if api is not present - return {} unless @api + api = @api + return {} unless api # TODO: id generation # TODO: runtime information is required for payload # TODO: return error response - use some wrapper from ddtrace as an example - @api.api_request( + api.api_request( path: Ext::Transport::DD_API_SETTINGS_PATH, payload: settings_payload(service: service) ) From 4842bba73360553eb8f7a5784c0a8188dcccdf7d Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Mon, 4 Mar 2024 17:09:47 +0100 Subject: [PATCH 11/20] pass TestSession object to the ApiClient together with runtime information included --- lib/datadog/ci/test_visibility/recorder.rb | 6 +++--- lib/datadog/ci/transport/api_client.rb | 4 ++-- sig/datadog/ci/test_visibility/recorder.rbs | 2 +- sig/datadog/ci/transport/api_client.rbs | 2 +- spec/datadog/ci/transport/api_client_spec.rb | 16 ++++------------ 5 files changed, 11 insertions(+), 19 deletions(-) diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index bf01e9a2..f8e70bab 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -56,7 +56,7 @@ def start_test_session(service: nil, tags: {}) test_session = build_test_session(tracer_span, tags) - configure_library(service: service) + configure_library(test_session) test_session end @@ -190,12 +190,12 @@ def itr_enabled? private - def configure_library(service:) + def configure_library(test_session) return unless itr_enabled? # TODO: error handling when request failed - disable ITR runner # TODO: need to pass runtime information # TODO: disable ITR runner if itr_enabled is false in settings - @api_client.fetch_library_settings(service: service) + @api_client.fetch_library_settings(test_session) end def skip_tracing(block = nil) diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/api_client.rb index 30ee2ec6..ffbe1971 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -15,7 +15,7 @@ def initialize(api: nil, dd_env: nil) @dd_env = dd_env end - def fetch_library_settings(service:) + def fetch_library_settings(test_session) # TODO: return error response if api is not present api = @api return {} unless api @@ -24,7 +24,7 @@ def fetch_library_settings(service:) # TODO: return error response - use some wrapper from ddtrace as an example api.api_request( path: Ext::Transport::DD_API_SETTINGS_PATH, - payload: settings_payload(service: service) + payload: settings_payload(service: test_session.service) ) end diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index a3e3ae94..357ef9f9 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -48,7 +48,7 @@ module Datadog private - def configure_library: (service: String?) -> void + def configure_library: (Datadog::CI::TestSession test_session) -> void def create_datadog_span: (String span_name, ?span_options: Hash[untyped, untyped], ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped diff --git a/sig/datadog/ci/transport/api_client.rbs b/sig/datadog/ci/transport/api_client.rbs index 06d3d7ab..77198a9f 100644 --- a/sig/datadog/ci/transport/api_client.rbs +++ b/sig/datadog/ci/transport/api_client.rbs @@ -7,7 +7,7 @@ module Datadog def initialize: (?api: Datadog::CI::Transport::Api::Base?, ?dd_env: String?) -> void - def fetch_library_settings: (service: String?) -> untyped + def fetch_library_settings: (Datadog::CI::TestSession test_session) -> untyped private diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/api_client_spec.rb index 94997db7..e380a9a4 100644 --- a/spec/datadog/ci/transport/api_client_spec.rb +++ b/spec/datadog/ci/transport/api_client_spec.rb @@ -8,21 +8,13 @@ describe "#fetch_library_settings" do let(:service) { "service" } + let(:tracer_span) { double("tracer_span", service: service) } + let(:test_session) { Datadog::CI::TestSession.new(tracer_span) } + let(:path) { Datadog::CI::Ext::Transport::DD_API_SETTINGS_PATH } - let(:payload) do - { - data: { - id: "change_me", - type: Datadog::CI::Ext::Transport::DD_API_SETTINGS_TYPE, - attributes: { - service: service - } - } - }.to_json - end it "requests the settings" do - subject.fetch_library_settings(service: service) + subject.fetch_library_settings(test_session) expect(api).to have_received(:api_request) do |args| expect(args[:path]).to eq(path) From 688f02ae9b68d948ecb621e1fb91a520995bc013 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 5 Mar 2024 09:23:30 +0100 Subject: [PATCH 12/20] send correct settings request payload --- lib/datadog/ci/span.rb | 42 +++++++++++++++ lib/datadog/ci/test_visibility/recorder.rb | 3 +- lib/datadog/ci/transport/api_client.rb | 29 ++++++---- sig/datadog/ci/span.rbs | 14 +++++ sig/datadog/ci/transport/api_client.rbs | 2 +- spec/datadog/ci/span_spec.rb | 56 ++++++++++++++++++++ spec/datadog/ci/transport/api_client_spec.rb | 23 +++++++- 7 files changed, 155 insertions(+), 14 deletions(-) diff --git a/lib/datadog/ci/span.rb b/lib/datadog/ci/span.rb index e09c9b52..481c87e1 100644 --- a/lib/datadog/ci/span.rb +++ b/lib/datadog/ci/span.rb @@ -121,6 +121,48 @@ def set_tags(tags) tracer_span.set_tags(tags) end + # Returns the git repository URL extracted from the environment. + # @return [String] the repository URL. + def git_repository_url + tracer_span.get_tag(Ext::Git::TAG_REPOSITORY_URL) + end + + # Returns the latest commit SHA extracted from the environment. + # @return [String] the commit SHA of the last commit. + def git_commit_sha + tracer_span.get_tag(Ext::Git::TAG_COMMIT_SHA) + end + + # Returns the git branch name extracted from the environment. + # @return [String] the branch. + def git_branch + tracer_span.get_tag(Ext::Git::TAG_BRANCH) + end + + # Returns the OS architecture extracted from the environment. + # @return [String] OS arch. + def os_architecture + tracer_span.get_tag(Ext::Test::TAG_OS_ARCHITECTURE) + end + + # Returns the OS platform extracted from the environment. + # @return [String] OS platform. + def os_platform + tracer_span.get_tag(Ext::Test::TAG_OS_PLATFORM) + end + + # Returns the runtime name extracted from the environment. + # @return [String] runtime name. + def runtime_name + tracer_span.get_tag(Ext::Test::TAG_RUNTIME_NAME) + end + + # Returns the runtime version extracted from the environment. + # @return [String] runtime version. + def runtime_version + tracer_span.get_tag(Ext::Test::TAG_RUNTIME_VERSION) + end + def set_environment_runtime_tags tracer_span.set_tag(Ext::Test::TAG_OS_ARCHITECTURE, ::RbConfig::CONFIG["host_cpu"]) tracer_span.set_tag(Ext::Test::TAG_OS_PLATFORM, ::RbConfig::CONFIG["host_os"]) diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index f8e70bab..9eed960a 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -193,8 +193,7 @@ def itr_enabled? def configure_library(test_session) return unless itr_enabled? # TODO: error handling when request failed - disable ITR runner - # TODO: need to pass runtime information - # TODO: disable ITR runner if itr_enabled is false in settings + # TODO: configure ITR runner based on response @api_client.fetch_library_settings(test_session) end diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/api_client.rb index ffbe1971..bda44da5 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -9,6 +9,8 @@ module CI module Transport # Datadog API client # Calls settings endpoint to fetch library settings for given service and env + # + # TODO: Rename ApiClient to SettingsApiClient class ApiClient def initialize(api: nil, dd_env: nil) @api = api @@ -19,25 +21,32 @@ def fetch_library_settings(test_session) # TODO: return error response if api is not present api = @api return {} unless api - # TODO: id generation - # TODO: runtime information is required for payload # TODO: return error response - use some wrapper from ddtrace as an example api.api_request( path: Ext::Transport::DD_API_SETTINGS_PATH, - payload: settings_payload(service: test_session.service) + payload: payload(test_session) ) end private - def settings_payload(service:) + def payload(test_session) { - data: { - id: Datadog::Core::Environment::Identity.id, - type: Ext::Transport::DD_API_SETTINGS_TYPE, - attributes: { - service: service, - env: @dd_env + "data" => { + "id" => Datadog::Core::Environment::Identity.id, + "type" => Ext::Transport::DD_API_SETTINGS_TYPE, + "attributes" => { + "service" => test_session.service, + "env" => @dd_env, + "repository_url" => test_session.git_repository_url, + "branch" => test_session.git_branch, + "sha" => test_session.git_commit_sha, + "configurations" => { + "os.platform" => test_session.os_platform, + "os.arch" => test_session.os_architecture, + "runtime.name" => test_session.runtime_name, + "runtime.version" => test_session.runtime_version + } } } }.to_json diff --git a/sig/datadog/ci/span.rbs b/sig/datadog/ci/span.rbs index 7dcf3f5d..cbf14fff 100644 --- a/sig/datadog/ci/span.rbs +++ b/sig/datadog/ci/span.rbs @@ -45,6 +45,20 @@ module Datadog def set_parameters: (Hash[String, Object] arguments, ?Hash[String, Object] metadata) -> void + def git_repository_url: () -> String? + + def git_commit_sha: () -> String? + + def git_branch: () -> String? + + def os_architecture: () -> String? + + def os_platform: () -> String? + + def runtime_name: () -> String? + + def runtime_version: () -> String? + private def recorder: () -> Datadog::CI::TestVisibility::Recorder diff --git a/sig/datadog/ci/transport/api_client.rbs b/sig/datadog/ci/transport/api_client.rbs index 77198a9f..e50bea89 100644 --- a/sig/datadog/ci/transport/api_client.rbs +++ b/sig/datadog/ci/transport/api_client.rbs @@ -11,7 +11,7 @@ module Datadog private - def settings_payload: (service: String?) -> String + def payload: (Datadog::CI::TestSession test_session) -> String end end end diff --git a/spec/datadog/ci/span_spec.rb b/spec/datadog/ci/span_spec.rb index 897b779a..b7709d34 100644 --- a/spec/datadog/ci/span_spec.rb +++ b/spec/datadog/ci/span_spec.rb @@ -212,4 +212,60 @@ expect(span.type).to eq("test") end end + + describe "#git_repository_url" do + it "returns the git repository URL" do + expect(tracer_span).to receive(:get_tag).with("git.repository_url").and_return("url") + + expect(span.git_repository_url).to eq("url") + end + end + + describe "#git_commit_sha" do + it "returns the git commit SHA" do + expect(tracer_span).to receive(:get_tag).with("git.commit.sha").and_return("sha") + + expect(span.git_commit_sha).to eq("sha") + end + end + + describe "#git_branch" do + it "returns the git branch" do + expect(tracer_span).to receive(:get_tag).with("git.branch").and_return("branch") + + expect(span.git_branch).to eq("branch") + end + end + + describe "#os_architecture" do + it "returns the OS architecture" do + expect(tracer_span).to receive(:get_tag).with("os.architecture").and_return("arch") + + expect(span.os_architecture).to eq("arch") + end + end + + describe "#os_platform" do + it "returns the OS platform" do + expect(tracer_span).to receive(:get_tag).with("os.platform").and_return("platform") + + expect(span.os_platform).to eq("platform") + end + end + + describe "#runtime_name" do + it "returns the runtime name" do + expect(tracer_span).to receive(:get_tag).with("runtime.name").and_return("name") + + expect(span.runtime_name).to eq("name") + end + end + + describe "#runtime_version" do + it "returns the runtime version" do + expect(tracer_span).to receive(:get_tag).with("runtime.version").and_return("version") + + expect(span.runtime_version).to eq("version") + end + end end diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/api_client_spec.rb index e380a9a4..c490ab4f 100644 --- a/spec/datadog/ci/transport/api_client_spec.rb +++ b/spec/datadog/ci/transport/api_client_spec.rb @@ -8,7 +8,19 @@ describe "#fetch_library_settings" do let(:service) { "service" } - let(:tracer_span) { double("tracer_span", service: service) } + let(:tracer_span) do + Datadog::Tracing::SpanOperation.new("session", service: service).tap do |span| + span.set_tags({ + "git.repository_url" => "repository_url", + "git.branch" => "branch", + "git.commit.sha" => "commit_sha", + "os.platform" => "platform", + "os.architecture" => "arch", + "runtime.name" => "runtime_name", + "runtime.version" => "runtime_version" + }) + end + end let(:test_session) { Datadog::CI::TestSession.new(tracer_span) } let(:path) { Datadog::CI::Ext::Transport::DD_API_SETTINGS_PATH } @@ -27,6 +39,15 @@ attributes = data["attributes"] expect(attributes["service"]).to eq(service) expect(attributes["env"]).to eq(dd_env) + expect(attributes["repository_url"]).to eq("repository_url") + expect(attributes["branch"]).to eq("branch") + expect(attributes["sha"]).to eq("commit_sha") + + configurations = attributes["configurations"] + expect(configurations["os.platform"]).to eq("platform") + expect(configurations["os.arch"]).to eq("arch") + expect(configurations["runtime.name"]).to eq("runtime_name") + expect(configurations["runtime.version"]).to eq("runtime_version") end end end From a5bc7565d5add4a616f86f4f18777872dd2f100a Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 5 Mar 2024 10:29:33 +0100 Subject: [PATCH 13/20] error handling for settings api --- lib/datadog/ci/transport/api_client.rb | 47 +++++++++-- sig/datadog/ci/transport/api_client.rbs | 15 +++- spec/datadog/ci/transport/api_client_spec.rb | 88 +++++++++++++++++++- 3 files changed, 141 insertions(+), 9 deletions(-) diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/api_client.rb index bda44da5..7f08ea8b 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "json" + require "datadog/core/environment/identity" require_relative "../ext/transport" @@ -12,19 +14,52 @@ module Transport # # TODO: Rename ApiClient to SettingsApiClient class ApiClient + class Response + def initialize(http_response) + @http_response = http_response + @json = nil + end + + def ok? + resp = @http_response + !resp.nil? && resp.ok? + end + + def payload + return @json if @json + + resp = @http_response + return default_payload if resp.nil? || !resp.ok? + + begin + @json = JSON.parse(resp.payload).dig("data", "attributes") + rescue JSON::ParserError => e + Datadog.logger.error("Failed to parse settings response payload: #{e}. Payload was: #{resp.payload}") + @json = default_payload + end + end + + private + + def default_payload + {"itr_enabled" => false} + end + end + def initialize(api: nil, dd_env: nil) @api = api @dd_env = dd_env end def fetch_library_settings(test_session) - # TODO: return error response if api is not present api = @api - return {} unless api - # TODO: return error response - use some wrapper from ddtrace as an example - api.api_request( - path: Ext::Transport::DD_API_SETTINGS_PATH, - payload: payload(test_session) + return Response.new(nil) unless api + + Response.new( + api.api_request( + path: Ext::Transport::DD_API_SETTINGS_PATH, + payload: payload(test_session) + ) ) end diff --git a/sig/datadog/ci/transport/api_client.rbs b/sig/datadog/ci/transport/api_client.rbs index e50bea89..7310df6c 100644 --- a/sig/datadog/ci/transport/api_client.rbs +++ b/sig/datadog/ci/transport/api_client.rbs @@ -2,12 +2,25 @@ module Datadog module CI module Transport class ApiClient + class Response + @http_response: Datadog::Core::Transport::HTTP::Adapters::Net::Response? + @json: Hash[String, untyped]? + + def initialize: (Datadog::Core::Transport::HTTP::Adapters::Net::Response? http_response) -> void + + def ok?: () -> bool + + private + + def default_payload: () -> Hash[String, untyped] + end + @api: Datadog::CI::Transport::Api::Base? @dd_env: String? def initialize: (?api: Datadog::CI::Transport::Api::Base?, ?dd_env: String?) -> void - def fetch_library_settings: (Datadog::CI::TestSession test_session) -> untyped + def fetch_library_settings: (Datadog::CI::TestSession test_session) -> Response private diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/api_client_spec.rb index c490ab4f..4acdfef0 100644 --- a/spec/datadog/ci/transport/api_client_spec.rb +++ b/spec/datadog/ci/transport/api_client_spec.rb @@ -4,7 +4,7 @@ let(:api) { spy("api") } let(:dd_env) { "ci" } - subject { described_class.new(api: api, dd_env: dd_env) } + subject(:client) { described_class.new(api: api, dd_env: dd_env) } describe "#fetch_library_settings" do let(:service) { "service" } @@ -26,7 +26,7 @@ let(:path) { Datadog::CI::Ext::Transport::DD_API_SETTINGS_PATH } it "requests the settings" do - subject.fetch_library_settings(test_session) + client.fetch_library_settings(test_session) expect(api).to have_received(:api_request) do |args| expect(args[:path]).to eq(path) @@ -50,5 +50,89 @@ expect(configurations["runtime.version"]).to eq("runtime_version") end end + + context "parsing response" do + subject(:response) { client.fetch_library_settings(test_session) } + + context "when api is present" do + before do + allow(api).to receive(:api_request).and_return(http_response) + end + + context "when response is OK" do + let(:http_response) do + double( + "http_response", + ok?: true, + payload: { + "data" => { + "id" => "123", + "type" => Datadog::CI::Ext::Transport::DD_API_SETTINGS_TYPE, + "attributes" => { + "code_coverage" => true, + "tests_skipping" => false, + "itr_enabled" => true, + "require_git" => false + } + } + }.to_json + ) + end + + it "parses the response" do + expect(response.ok?).to be true + expect(response.payload).to eq({ + "code_coverage" => true, + "tests_skipping" => false, + "itr_enabled" => true, + "require_git" => false + }) + end + end + + context "when response is not OK" do + let(:http_response) do + double( + "http_response", + ok?: false, + payload: "" + ) + end + + it "parses the response" do + expect(response.ok?).to be false + expect(response.payload).to eq("itr_enabled" => false) + end + end + + context "when response is OK but JSON is malformed" do + let(:http_response) do + double( + "http_response", + ok?: true, + payload: "not json" + ) + end + + before do + expect(Datadog.logger).to receive(:error).with(/Failed to parse settings response payload/) + end + + it "parses the response" do + expect(response.ok?).to be true + expect(response.payload).to eq("itr_enabled" => false) + end + end + end + + context "when there is no api" do + let(:api) { nil } + + it "returns an empty response" do + expect(response.ok?).to be false + expect(response.payload).to eq("itr_enabled" => false) + end + end + end end end From c52453e1dd8598dff23c39a2746bee96c52cbb90 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 5 Mar 2024 12:41:13 +0100 Subject: [PATCH 14/20] configure ITR::Runner according to remote configuration --- lib/datadog/ci/ext/transport.rb | 6 +++ lib/datadog/ci/itr/runner.rb | 30 ++++++++++++- lib/datadog/ci/test_visibility/recorder.rb | 7 +-- lib/datadog/ci/transport/api_client.rb | 10 +++-- sig/datadog/ci/ext/transport.rbs | 12 +++++ sig/datadog/ci/itr/runner.rbs | 12 ++++- sig/datadog/ci/transport/api_client.rbs | 2 + spec/datadog/ci/itr/runner_spec.rb | 46 ++++++++++++++++++-- spec/datadog/ci/transport/api_client_spec.rb | 22 ++++++++++ 9 files changed, 134 insertions(+), 13 deletions(-) diff --git a/lib/datadog/ci/ext/transport.rb b/lib/datadog/ci/ext/transport.rb index 93d34f6e..c6507729 100644 --- a/lib/datadog/ci/ext/transport.rb +++ b/lib/datadog/ci/ext/transport.rb @@ -26,6 +26,12 @@ module Transport DD_API_HOST_PREFIX = "api" DD_API_SETTINGS_PATH = "/api/v2/ci/libraries/tests/services/setting" DD_API_SETTINGS_TYPE = "ci_app_test_service_libraries_settings" + DD_API_SETTINGS_RESPONSE_DIG_KEYS = %w[data attributes].freeze + DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY = "itr_enabled" + DD_API_SETTINGS_RESPONSE_CODE_COVERAGE_KEY = "code_coverage" + DD_API_SETTINGS_RESPONSE_TESTS_SKIPPING_KEY = "tests_skipping" + DD_API_SETTINGS_RESPONSE_REQUIRE_GIT_KEY = "require_git" + DD_API_SETTINGS_RESPONSE_DEFAULT = {DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY => false}.freeze CONTENT_TYPE_MESSAGEPACK = "application/msgpack" CONTENT_TYPE_JSON = "application/json" diff --git a/lib/datadog/ci/itr/runner.rb b/lib/datadog/ci/itr/runner.rb index 00b46537..d65bfe65 100644 --- a/lib/datadog/ci/itr/runner.rb +++ b/lib/datadog/ci/itr/runner.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "../ext/transport" + module Datadog module CI module ITR @@ -11,14 +13,38 @@ def initialize( enabled: false ) @enabled = enabled + @test_skipping_enabled = false + @code_coverage_enabled = false + end + + def configure(remote_configuration) + @enabled = convert_to_bool( + remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY, false) + ) + @test_skipping_enabled = @enabled && convert_to_bool( + remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_TESTS_SKIPPING_KEY, false) + ) + @code_coverage_enabled = @enabled && convert_to_bool( + remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_CODE_COVERAGE_KEY, false) + ) end def enabled? @enabled end - def disable - @enabled = false + def skipping_tests? + @test_skipping_enabled + end + + def code_coverage? + @code_coverage_enabled + end + + private + + def convert_to_bool(value) + value.to_s == "true" end end end diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index 9eed960a..8e8a9fbf 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -191,10 +191,11 @@ def itr_enabled? private def configure_library(test_session) + # this will change when EFD is implemented return unless itr_enabled? - # TODO: error handling when request failed - disable ITR runner - # TODO: configure ITR runner based on response - @api_client.fetch_library_settings(test_session) + + remote_configuration = @api_client.fetch_library_settings(test_session) + @itr.configure(remote_configuration.payload) end def skip_tracing(block = nil) diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/api_client.rb index 7f08ea8b..dad33840 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/api_client.rb @@ -26,13 +26,15 @@ def ok? end def payload - return @json if @json + cached = @json + return cached unless cached.nil? resp = @http_response - return default_payload if resp.nil? || !resp.ok? + return @json = default_payload if resp.nil? || !resp.ok? begin - @json = JSON.parse(resp.payload).dig("data", "attributes") + @json = JSON.parse(resp.payload).dig(*Ext::Transport::DD_API_SETTINGS_RESPONSE_DIG_KEYS) || + default_payload rescue JSON::ParserError => e Datadog.logger.error("Failed to parse settings response payload: #{e}. Payload was: #{resp.payload}") @json = default_payload @@ -42,7 +44,7 @@ def payload private def default_payload - {"itr_enabled" => false} + Ext::Transport::DD_API_SETTINGS_RESPONSE_DEFAULT end end diff --git a/sig/datadog/ci/ext/transport.rbs b/sig/datadog/ci/ext/transport.rbs index 75a9cd35..59c9a935 100644 --- a/sig/datadog/ci/ext/transport.rbs +++ b/sig/datadog/ci/ext/transport.rbs @@ -32,6 +32,18 @@ module Datadog DD_API_SETTINGS_TYPE: "ci_app_test_service_libraries_settings" + DD_API_SETTINGS_RESPONSE_DIG_KEYS: Array[String] + + DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY: "itr_enabled" + + DD_API_SETTINGS_RESPONSE_CODE_COVERAGE_KEY: "code_coverage" + + DD_API_SETTINGS_RESPONSE_TESTS_SKIPPING_KEY: "tests_skipping" + + DD_API_SETTINGS_RESPONSE_REQUIRE_GIT_KEY: "require_git" + + DD_API_SETTINGS_RESPONSE_DEFAULT: Hash[String, untyped] + CONTENT_TYPE_MESSAGEPACK: "application/msgpack" CONTENT_TYPE_JSON: "application/json" diff --git a/sig/datadog/ci/itr/runner.rbs b/sig/datadog/ci/itr/runner.rbs index 37293e6c..7a575bdb 100644 --- a/sig/datadog/ci/itr/runner.rbs +++ b/sig/datadog/ci/itr/runner.rbs @@ -3,12 +3,22 @@ module Datadog module ITR class Runner @enabled: bool + @test_skipping_enabled: bool + @code_coverage_enabled: bool def initialize: (?enabled: bool) -> void + def configure: (Hash[String, untyped] remote_configuration) -> void + def enabled?: () -> bool - def disable: () -> void + def skipping_tests?: () -> bool + + def code_coverage: () -> bool + + private + + def convert_to_bool: (untyped value) -> bool end end end diff --git a/sig/datadog/ci/transport/api_client.rbs b/sig/datadog/ci/transport/api_client.rbs index 7310df6c..41b102f4 100644 --- a/sig/datadog/ci/transport/api_client.rbs +++ b/sig/datadog/ci/transport/api_client.rbs @@ -10,6 +10,8 @@ module Datadog def ok?: () -> bool + def payload: () -> Hash[String, untyped] + private def default_payload: () -> Hash[String, untyped] diff --git a/spec/datadog/ci/itr/runner_spec.rb b/spec/datadog/ci/itr/runner_spec.rb index fa6c90d0..39f6a29a 100644 --- a/spec/datadog/ci/itr/runner_spec.rb +++ b/spec/datadog/ci/itr/runner_spec.rb @@ -7,9 +7,49 @@ subject(:runner) { described_class.new(enabled: itr_enabled) } - describe "#disable" do - it "disables the runner" do - expect { runner.disable }.to change { runner.enabled? }.from(true).to(false) + describe "#configure" do + before do + runner.configure(remote_configuration) + end + + context "when remote configuration call failed" do + let(:remote_configuration) { {"itr_enabled" => false} } + + it "configures the runner" do + expect(runner.enabled?).to be false + expect(runner.skipping_tests?).to be false + expect(runner.code_coverage?).to be false + end + end + + context "when remote configuration call returned correct response" do + let(:remote_configuration) { {"itr_enabled" => true, "code_coverage" => true, "tests_skipping" => false} } + + it "configures the runner" do + expect(runner.enabled?).to be true + expect(runner.skipping_tests?).to be false + expect(runner.code_coverage?).to be true + end + end + + context "when remote configuration call returned correct response with strings instead of bools" do + let(:remote_configuration) { {"itr_enabled" => "true", "code_coverage" => "true", "tests_skipping" => "false"} } + + it "configures the runner" do + expect(runner.enabled?).to be true + expect(runner.skipping_tests?).to be false + expect(runner.code_coverage?).to be true + end + end + + context "when remote configuration call returns empty hash" do + let(:remote_configuration) { {} } + + it "configures the runner" do + expect(runner.enabled?).to be false + expect(runner.skipping_tests?).to be false + expect(runner.code_coverage?).to be false + end end end end diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/api_client_spec.rb index 4acdfef0..6b04e21d 100644 --- a/spec/datadog/ci/transport/api_client_spec.rb +++ b/spec/datadog/ci/transport/api_client_spec.rb @@ -123,6 +123,28 @@ expect(response.payload).to eq("itr_enabled" => false) end end + + context "when response is OK but JSON has different format" do + let(:http_response) do + double( + "http_response", + ok?: true, + payload: { + "attributes" => { + "code_coverage" => true, + "tests_skipping" => false, + "itr_enabled" => true, + "require_git" => false + } + }.to_json + ) + end + + it "parses the response" do + expect(response.ok?).to be true + expect(response.payload).to eq("itr_enabled" => false) + end + end end context "when there is no api" do From 04be43da35a947bbe6d233dd500b687d149e9bc3 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 5 Mar 2024 12:47:43 +0100 Subject: [PATCH 15/20] rename ApiClient to RemoteSettingsApi --- lib/datadog/ci/configuration/components.rb | 5 +++-- lib/datadog/ci/test_visibility/recorder.rb | 7 +++---- .../ci/transport/{api_client.rb => remote_settings_api.rb} | 4 +--- sig/datadog/ci/test_visibility/recorder.rbs | 4 ++-- .../transport/{api_client.rbs => remote_settings_api.rbs} | 2 +- .../{api_client_spec.rb => remote_settings_api_spec.rb} | 4 ++-- 6 files changed, 12 insertions(+), 14 deletions(-) rename lib/datadog/ci/transport/{api_client.rb => remote_settings_api.rb} (96%) rename sig/datadog/ci/transport/{api_client.rbs => remote_settings_api.rbs} (96%) rename spec/datadog/ci/transport/{api_client_spec.rb => remote_settings_api_spec.rb} (97%) diff --git a/lib/datadog/ci/configuration/components.rb b/lib/datadog/ci/configuration/components.rb index 345f256d..9977a401 100644 --- a/lib/datadog/ci/configuration/components.rb +++ b/lib/datadog/ci/configuration/components.rb @@ -9,6 +9,7 @@ require_relative "../test_visibility/serializers/factories/test_suite_level" require_relative "../test_visibility/transport" require_relative "../transport/api/builder" +require_relative "../transport/remote_settings_api" module Datadog module CI @@ -74,7 +75,7 @@ def activate_ci!(settings) enabled: settings.ci.enabled && settings.ci.itr_enabled ) - api_client = Transport::ApiClient.new( + remote_settings_api = Transport::RemoteSettingsApi.new( api: test_visibility_api, dd_env: settings.env ) @@ -83,7 +84,7 @@ def activate_ci!(settings) @ci_recorder = TestVisibility::Recorder.new( test_suite_level_visibility_enabled: !settings.ci.force_test_level_visibility, itr: itr, - api_client: api_client + remote_settings_api: remote_settings_api ) end diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index 8e8a9fbf..83459e11 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -12,7 +12,6 @@ require_relative "../ext/app_types" require_relative "../ext/test" require_relative "../ext/environment" -require_relative "../transport/api_client" require_relative "../utils/git" require_relative "../span" @@ -30,7 +29,7 @@ class Recorder attr_reader :environment_tags, :test_suite_level_visibility_enabled def initialize( - itr:, api_client:, test_suite_level_visibility_enabled: false, + itr:, remote_settings_api:, test_suite_level_visibility_enabled: false, codeowners: Codeowners::Parser.new(Utils::Git.root).parse ) @test_suite_level_visibility_enabled = test_suite_level_visibility_enabled @@ -42,7 +41,7 @@ def initialize( @codeowners = codeowners @itr = itr - @api_client = api_client + @remote_settings_api = remote_settings_api end def start_test_session(service: nil, tags: {}) @@ -194,7 +193,7 @@ def configure_library(test_session) # this will change when EFD is implemented return unless itr_enabled? - remote_configuration = @api_client.fetch_library_settings(test_session) + remote_configuration = @remote_settings_api.fetch_library_settings(test_session) @itr.configure(remote_configuration.payload) end diff --git a/lib/datadog/ci/transport/api_client.rb b/lib/datadog/ci/transport/remote_settings_api.rb similarity index 96% rename from lib/datadog/ci/transport/api_client.rb rename to lib/datadog/ci/transport/remote_settings_api.rb index dad33840..a8c0432b 100644 --- a/lib/datadog/ci/transport/api_client.rb +++ b/lib/datadog/ci/transport/remote_settings_api.rb @@ -11,9 +11,7 @@ module CI module Transport # Datadog API client # Calls settings endpoint to fetch library settings for given service and env - # - # TODO: Rename ApiClient to SettingsApiClient - class ApiClient + class RemoteSettingsApi class Response def initialize(http_response) @http_response = http_response diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index 357ef9f9..1f1c96a2 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -8,13 +8,13 @@ module Datadog @local_context: Datadog::CI::TestVisibility::Context::Local @global_context: Datadog::CI::TestVisibility::Context::Global @itr: Datadog::CI::ITR::Runner - @api_client: Datadog::CI::Transport::ApiClient + @remote_settings_api: Datadog::CI::Transport::RemoteSettingsApi @codeowners: Datadog::CI::Codeowners::Matcher attr_reader environment_tags: Hash[String, String] attr_reader test_suite_level_visibility_enabled: bool - def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher, itr: Datadog::CI::ITR::Runner, api_client: Datadog::CI::Transport::ApiClient) -> void + def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher, itr: Datadog::CI::ITR::Runner, remote_settings_api: Datadog::CI::Transport::RemoteSettingsApi) -> void def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Test span) -> untyped } -> untyped diff --git a/sig/datadog/ci/transport/api_client.rbs b/sig/datadog/ci/transport/remote_settings_api.rbs similarity index 96% rename from sig/datadog/ci/transport/api_client.rbs rename to sig/datadog/ci/transport/remote_settings_api.rbs index 41b102f4..cbf422a3 100644 --- a/sig/datadog/ci/transport/api_client.rbs +++ b/sig/datadog/ci/transport/remote_settings_api.rbs @@ -1,7 +1,7 @@ module Datadog module CI module Transport - class ApiClient + class RemoteSettingsApi class Response @http_response: Datadog::Core::Transport::HTTP::Adapters::Net::Response? @json: Hash[String, untyped]? diff --git a/spec/datadog/ci/transport/api_client_spec.rb b/spec/datadog/ci/transport/remote_settings_api_spec.rb similarity index 97% rename from spec/datadog/ci/transport/api_client_spec.rb rename to spec/datadog/ci/transport/remote_settings_api_spec.rb index 6b04e21d..651a47e9 100644 --- a/spec/datadog/ci/transport/api_client_spec.rb +++ b/spec/datadog/ci/transport/remote_settings_api_spec.rb @@ -1,6 +1,6 @@ -require_relative "../../../../lib/datadog/ci/transport/api_client" +require_relative "../../../../lib/datadog/ci/transport/remote_settings_api" -RSpec.describe Datadog::CI::Transport::ApiClient do +RSpec.describe Datadog::CI::Transport::RemoteSettingsApi do let(:api) { spy("api") } let(:dd_env) { "ci" } From fb76b51f95fa29d2eb26befa85c34ea15fffef0a Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 5 Mar 2024 13:49:21 +0100 Subject: [PATCH 16/20] add ITR tags to TestSession object when configuring ITR --- lib/datadog/ci/ext/test.rb | 9 + lib/datadog/ci/itr/runner.rb | 11 +- lib/datadog/ci/test_session.rb | 8 + lib/datadog/ci/test_visibility/recorder.rb | 2 +- sig/datadog/ci/ext/test.rbs | 8 + sig/datadog/ci/itr/runner.rbs | 2 +- spec/datadog/ci/itr/runner_spec.rb | 15 +- spec/datadog/ci/test_session_spec.rb | 53 +- .../ci/test_visibility/recorder_spec.rb | 810 +++++++++--------- spec/support/contexts/ci_mode.rb | 2 + 10 files changed, 522 insertions(+), 398 deletions(-) diff --git a/lib/datadog/ci/ext/test.rb b/lib/datadog/ci/ext/test.rb index b5bbea31..c3f33922 100644 --- a/lib/datadog/ci/ext/test.rb +++ b/lib/datadog/ci/ext/test.rb @@ -23,6 +23,13 @@ module Test TAG_CODEOWNERS = "test.codeowners" TAG_PARAMETERS = "test.parameters" + # ITR tags + TAG_ITR_TEST_SKIPPING_ENABLED = "test.itr.tests_skipping.enabled" + TAG_ITR_TEST_SKIPPING_TYPE = "test.itr.tests_skipping.type" + + # Code coverage tags + TAG_CODE_COVERAGE_ENABLED = "test.code_coverage.enabled" + # those tags are special and used to correlate tests with the test sessions, suites, and modules # they are transient and not sent to the backend TAG_TEST_SESSION_ID = "_test.session_id" @@ -43,6 +50,8 @@ module Test TAG_SPAN_KIND = "span.kind" SPAN_KIND_TEST = "test" + ITR_TEST_SKIPPING_MODE = "test" + # test status as recognized by Datadog module Status PASS = "pass" diff --git a/lib/datadog/ci/itr/runner.rb b/lib/datadog/ci/itr/runner.rb index d65bfe65..698719b5 100644 --- a/lib/datadog/ci/itr/runner.rb +++ b/lib/datadog/ci/itr/runner.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require_relative "../ext/test" require_relative "../ext/transport" module Datadog @@ -17,7 +18,7 @@ def initialize( @code_coverage_enabled = false end - def configure(remote_configuration) + def configure(remote_configuration, test_session) @enabled = convert_to_bool( remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY, false) ) @@ -27,6 +28,14 @@ def configure(remote_configuration) @code_coverage_enabled = @enabled && convert_to_bool( remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_CODE_COVERAGE_KEY, false) ) + + test_session.set_tag(Ext::Test::TAG_ITR_TEST_SKIPPING_ENABLED, @test_skipping_enabled) + # currently we set this tag when ITR requires collecting code coverage + # this will change as soon as we implement total code coverage support in this library + test_session.set_tag(Ext::Test::TAG_CODE_COVERAGE_ENABLED, @code_coverage_enabled) + + # we skip tests, not suites + test_session.set_tag(Ext::Test::TAG_ITR_TEST_SKIPPING_TYPE, Ext::Test::ITR_TEST_SKIPPING_MODE) end def enabled? diff --git a/lib/datadog/ci/test_session.rb b/lib/datadog/ci/test_session.rb index 354abf1b..7ffdefbc 100644 --- a/lib/datadog/ci/test_session.rb +++ b/lib/datadog/ci/test_session.rb @@ -26,6 +26,14 @@ def name get_tag(Ext::Test::TAG_COMMAND) end + def skipping_tests? + get_tag(Ext::Test::TAG_ITR_TEST_SKIPPING_ENABLED) == "true" + end + + def code_coverage? + get_tag(Ext::Test::TAG_CODE_COVERAGE_ENABLED) == "true" + end + # Return the test session tags that could be inherited by sub-spans # @return [Hash] the tags to be inherited by sub-spans. def inheritable_tags diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index 83459e11..29330254 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -194,7 +194,7 @@ def configure_library(test_session) return unless itr_enabled? remote_configuration = @remote_settings_api.fetch_library_settings(test_session) - @itr.configure(remote_configuration.payload) + @itr.configure(remote_configuration.payload, test_session) end def skip_tracing(block = nil) diff --git a/sig/datadog/ci/ext/test.rbs b/sig/datadog/ci/ext/test.rbs index a2c6588b..5b235a54 100644 --- a/sig/datadog/ci/ext/test.rbs +++ b/sig/datadog/ci/ext/test.rbs @@ -32,6 +32,12 @@ module Datadog TAG_PARAMETERS: "test.parameters" + TAG_ITR_TEST_SKIPPING_ENABLED: "test.itr.tests_skipping.enabled" + + TAG_ITR_TEST_SKIPPING_TYPE: "test.itr.tests_skipping.type" + + TAG_CODE_COVERAGE_ENABLED: "test.code_coverage.enabled" + TAG_TEST_SESSION_ID: "_test.session_id" TAG_TEST_MODULE_ID: "_test.module_id" @@ -54,6 +60,8 @@ module Datadog SPAN_KIND_TEST: "test" + ITR_TEST_SKIPPING_MODE: "test" + module Status PASS: "pass" diff --git a/sig/datadog/ci/itr/runner.rbs b/sig/datadog/ci/itr/runner.rbs index 7a575bdb..1944eeb7 100644 --- a/sig/datadog/ci/itr/runner.rbs +++ b/sig/datadog/ci/itr/runner.rbs @@ -8,7 +8,7 @@ module Datadog def initialize: (?enabled: bool) -> void - def configure: (Hash[String, untyped] remote_configuration) -> void + def configure: (Hash[String, untyped] remote_configuration, Datadog::CI::TestSession test_session) -> void def enabled?: () -> bool diff --git a/spec/datadog/ci/itr/runner_spec.rb b/spec/datadog/ci/itr/runner_spec.rb index 39f6a29a..630e8ee9 100644 --- a/spec/datadog/ci/itr/runner_spec.rb +++ b/spec/datadog/ci/itr/runner_spec.rb @@ -8,14 +8,17 @@ subject(:runner) { described_class.new(enabled: itr_enabled) } describe "#configure" do + let(:tracer_span) { Datadog::Tracing::SpanOperation.new("session") } + let(:test_session) { Datadog::CI::TestSession.new(tracer_span) } + before do - runner.configure(remote_configuration) + runner.configure(remote_configuration, test_session) end context "when remote configuration call failed" do let(:remote_configuration) { {"itr_enabled" => false} } - it "configures the runner" do + it "configures the runner and test session" do expect(runner.enabled?).to be false expect(runner.skipping_tests?).to be false expect(runner.code_coverage?).to be false @@ -30,6 +33,14 @@ expect(runner.skipping_tests?).to be false expect(runner.code_coverage?).to be true end + + it "sets test session tags" do + expect(test_session.skipping_tests?).to be false + expect(test_session.code_coverage?).to be true + expect(test_session.get_tag(Datadog::CI::Ext::Test::TAG_ITR_TEST_SKIPPING_TYPE)).to eq( + Datadog::CI::Ext::Test::ITR_TEST_SKIPPING_MODE + ) + end end context "when remote configuration call returned correct response with strings instead of bools" do diff --git a/spec/datadog/ci/test_session_spec.rb b/spec/datadog/ci/test_session_spec.rb index 52ce638e..c20a18ec 100644 --- a/spec/datadog/ci/test_session_spec.rb +++ b/spec/datadog/ci/test_session_spec.rb @@ -1,14 +1,13 @@ # frozen_string_literal: true RSpec.describe Datadog::CI::TestSession do - let(:tracer_span) { instance_double(Datadog::Tracing::SpanOperation, finish: true) } + let(:tracer_span) { Datadog::Tracing::SpanOperation.new("session") } let(:recorder) { spy("recorder") } before { allow_any_instance_of(described_class).to receive(:recorder).and_return(recorder) } + subject(:ci_test_session) { described_class.new(tracer_span) } describe "#finish" do - subject(:ci_test_session) { described_class.new(tracer_span) } - it "deactivates the test session" do ci_test_session.finish @@ -19,11 +18,9 @@ describe "#inheritable_tags" do subject(:inheritable_tags) { ci_test_session.inheritable_tags } - let(:ci_test_session) { described_class.new(tracer_span) } - before do Datadog::CI::Ext::Test::INHERITABLE_TAGS.each do |tag| - allow(tracer_span).to receive(:get_tag).with(tag).and_return("value for #{tag}") + tracer_span.set_tag(tag, "value for #{tag}") end end @@ -39,12 +36,50 @@ describe "#name" do subject(:name) { ci_test_session.name } - let(:ci_test_session) { described_class.new(tracer_span) } - before do - allow(tracer_span).to receive(:get_tag).with(Datadog::CI::Ext::Test::TAG_COMMAND).and_return("test command") + tracer_span.set_tag(Datadog::CI::Ext::Test::TAG_COMMAND, "test command") end it { is_expected.to eq("test command") } end + + describe "#skipping_tests?" do + subject(:skipping_tests?) { ci_test_session.skipping_tests? } + + context "when not set" do + it { is_expected.to be false } + end + + context "when true" do + before { tracer_span.set_tag(Datadog::CI::Ext::Test::TAG_ITR_TEST_SKIPPING_ENABLED, true) } + + it { is_expected.to be true } + end + + context "when false" do + before { tracer_span.set_tag(Datadog::CI::Ext::Test::TAG_ITR_TEST_SKIPPING_ENABLED, false) } + + it { is_expected.to be false } + end + end + + describe "#code_coverage?" do + subject(:code_coverage?) { ci_test_session.code_coverage? } + + context "when not set" do + it { is_expected.to be false } + end + + context "when true" do + before { tracer_span.set_tag(Datadog::CI::Ext::Test::TAG_CODE_COVERAGE_ENABLED, true) } + + it { is_expected.to be true } + end + + context "when false" do + before { tracer_span.set_tag(Datadog::CI::Ext::Test::TAG_CODE_COVERAGE_ENABLED, false) } + + it { is_expected.to be false } + end + end end diff --git a/spec/datadog/ci/test_visibility/recorder_spec.rb b/spec/datadog/ci/test_visibility/recorder_spec.rb index 4f074867..da2d558e 100644 --- a/spec/datadog/ci/test_visibility/recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/recorder_spec.rb @@ -112,153 +112,108 @@ end context "when test suite level visibility is enabled" do - include_context "CI mode activated" + context "without ITR" do + include_context "CI mode activated" - describe "#trace" do - let(:type) { "step" } - let(:span_name) { "my test step" } - let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } + describe "#trace" do + let(:type) { "step" } + let(:span_name) { "my test step" } + let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } - context "when given a block" do - before do - recorder.trace(span_name, type: type, tags: tags) do |span| - span.set_metric("my.metric", 42) + context "when given a block" do + before do + recorder.trace(span_name, type: type, tags: tags) do |span| + span.set_metric("my.metric", 42) + end end - end - subject { span } - - it "traces the block" do - expect(subject.resource).to eq(span_name) - expect(subject.type).to eq(type) - end - - it "sets the custom metric correctly" do - expect(subject.get_metric("my.metric")).to eq(42) - end + subject { span } - it "sets the tags correctly" do - expect(subject).to have_test_tag("test.framework", "my-framework") - expect(subject).to have_test_tag("my.tag", "my_value") - end - - it_behaves_like "span with environment tags" - it_behaves_like "span with default tags" - it_behaves_like "span with runtime tags" - end - - context "without a block" do - subject { recorder.trace("my test step", type: type, tags: tags) } - - it "returns a new CI span" do - expect(subject).to be_kind_of(Datadog::CI::Span) - end + it "traces the block" do + expect(subject.resource).to eq(span_name) + expect(subject.type).to eq(type) + end - it "sets the tags correctly" do - expect(subject).to have_test_tag("test.framework", "my-framework") - expect(subject).to have_test_tag("my.tag", "my_value") - end + it "sets the custom metric correctly" do + expect(subject.get_metric("my.metric")).to eq(42) + end - it "sets correct resource and span type for the underlying tracer span" do - subject.finish + it "sets the tags correctly" do + expect(subject).to have_test_tag("test.framework", "my-framework") + expect(subject).to have_test_tag("my.tag", "my_value") + end - expect(span.resource).to eq(span_name) - expect(span.type).to eq(type) + it_behaves_like "span with environment tags" + it_behaves_like "span with default tags" + it_behaves_like "span with runtime tags" end - it_behaves_like "span with environment tags" - it_behaves_like "span with default tags" - it_behaves_like "span with runtime tags" - end - end - - describe "#trace_test" do - let(:test_name) { "my test" } - let(:test_suite_name) { "my suite" } - let(:test_service) { "my-service" } - let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } + context "without a block" do + subject { recorder.trace("my test step", type: type, tags: tags) } - context "without a block" do - subject do - recorder.trace_test( - test_name, - test_suite_name, - service: test_service, - tags: tags - ) - end - - context "when there is no active test session" do - it "returns a new CI test span" do - expect(subject).to be_kind_of(Datadog::CI::Test) - expect(subject.name).to eq(test_name) - expect(subject.service).to eq(test_service) - expect(subject.tracer_span.name).to eq(test_name) - expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST) + it "returns a new CI span" do + expect(subject).to be_kind_of(Datadog::CI::Span) end - it "sets the provided tags correctly" do + it "sets the tags correctly" do expect(subject).to have_test_tag("test.framework", "my-framework") expect(subject).to have_test_tag("my.tag", "my_value") end - it "does not connect the test span to the test session" do - expect(subject).not_to have_test_tag(:test_session_id) - end + it "sets correct resource and span type for the underlying tracer span" do + subject.finish - it "sets the test suite name as one of the tags" do - expect(subject).to have_test_tag(:suite, test_suite_name) - expect(subject).not_to have_test_tag(:test_suite_id) + expect(span.resource).to eq(span_name) + expect(span.type).to eq(type) end it_behaves_like "span with environment tags" it_behaves_like "span with default tags" it_behaves_like "span with runtime tags" - it_behaves_like "trace with ciapp-test origin" do - let(:trace_under_test) do - subject.finish - - trace - end - end end + end - context "when there is an active test session" do - let(:test_session_tags) { {"test.framework_version" => "1.0", "my.session.tag" => "my_session_value"} } - let(:session_service) { "my-session-service" } - let(:test_service) { nil } - - let(:test_session) { recorder.start_test_session(service: session_service, tags: test_session_tags) } + describe "#trace_test" do + let(:test_name) { "my test" } + let(:test_suite_name) { "my suite" } + let(:test_service) { "my-service" } + let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } - before do - test_session + context "without a block" do + subject do + recorder.trace_test( + test_name, + test_suite_name, + service: test_service, + tags: tags + ) end - context "when there is no active test module" do - it "returns a new CI test span using service from the test session" do + context "when there is no active test session" do + it "returns a new CI test span" do expect(subject).to be_kind_of(Datadog::CI::Test) expect(subject.name).to eq(test_name) - expect(subject.service).to eq(session_service) + expect(subject.service).to eq(test_service) + expect(subject.tracer_span.name).to eq(test_name) + expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST) end - it "sets the provided tags correctly while inheriting some tags from the session" do + it "sets the provided tags correctly" do expect(subject).to have_test_tag("test.framework", "my-framework") - expect(subject).to have_test_tag("test.framework_version", "1.0") expect(subject).to have_test_tag("my.tag", "my_value") - expect(subject).not_to have_test_tag("my.session.tag") end - it "connects the test span to the test session" do - expect(subject).to have_test_tag(:test_session_id, test_session.id.to_s) + it "does not connect the test span to the test session" do + expect(subject).not_to have_test_tag(:test_session_id) end - it "starts a new trace" do - expect(subject.tracer_span.trace_id).not_to eq(test_session.tracer_span.trace_id) + it "sets the test suite name as one of the tags" do + expect(subject).to have_test_tag(:suite, test_suite_name) + expect(subject).not_to have_test_tag(:test_suite_id) end it_behaves_like "span with environment tags" it_behaves_like "span with default tags" it_behaves_like "span with runtime tags" - it_behaves_like "trace with ciapp-test origin" do let(:trace_under_test) do subject.finish @@ -268,69 +223,152 @@ end end - context "when there is an active test module" do - let(:module_name) { "my-module" } + context "when there is an active test session" do + let(:test_session_tags) { {"test.framework_version" => "1.0", "my.session.tag" => "my_session_value"} } + let(:session_service) { "my-session-service" } + let(:test_service) { nil } - let(:test_module) do - recorder.start_test_module(module_name) - end + let(:test_session) { recorder.start_test_session(service: session_service, tags: test_session_tags) } before do - test_module + test_session end - it "returns a new CI test span" do - expect(subject).to be_kind_of(Datadog::CI::Test) - expect(subject.name).to eq(test_name) - end + context "when there is no active test module" do + it "returns a new CI test span using service from the test session" do + expect(subject).to be_kind_of(Datadog::CI::Test) + expect(subject.name).to eq(test_name) + expect(subject.service).to eq(session_service) + end - it "sets the provided tags correctly while inheriting some tags from the session" do - expect(subject).to have_test_tag("test.framework", "my-framework") - expect(subject).to have_test_tag("test.framework_version", "1.0") - expect(subject).to have_test_tag("my.tag", "my_value") - end + it "sets the provided tags correctly while inheriting some tags from the session" do + expect(subject).to have_test_tag("test.framework", "my-framework") + expect(subject).to have_test_tag("test.framework_version", "1.0") + expect(subject).to have_test_tag("my.tag", "my_value") + expect(subject).not_to have_test_tag("my.session.tag") + end + + it "connects the test span to the test session" do + expect(subject).to have_test_tag(:test_session_id, test_session.id.to_s) + end - it "connects the test span to the test module" do - expect(subject).to have_test_tag(:test_module_id, test_module.id.to_s) - expect(subject).to have_test_tag(:module, module_name) + it "starts a new trace" do + expect(subject.tracer_span.trace_id).not_to eq(test_session.tracer_span.trace_id) + end + + it_behaves_like "span with environment tags" + it_behaves_like "span with default tags" + it_behaves_like "span with runtime tags" + + it_behaves_like "trace with ciapp-test origin" do + let(:trace_under_test) do + subject.finish + + trace + end + end end - context "when there is an active test suite" do - let(:test_suite) do - recorder.start_test_suite(test_suite_name) + context "when there is an active test module" do + let(:module_name) { "my-module" } + + let(:test_module) do + recorder.start_test_module(module_name) end before do - test_suite + test_module + end + + it "returns a new CI test span" do + expect(subject).to be_kind_of(Datadog::CI::Test) + expect(subject.name).to eq(test_name) end - it "connects the test span to the test suite" do - expect(subject).to have_test_tag(:test_suite_id, test_suite.id.to_s) - expect(subject).to have_test_tag(:suite, test_suite_name) + it "sets the provided tags correctly while inheriting some tags from the session" do + expect(subject).to have_test_tag("test.framework", "my-framework") + expect(subject).to have_test_tag("test.framework_version", "1.0") + expect(subject).to have_test_tag("my.tag", "my_value") + end + + it "connects the test span to the test module" do + expect(subject).to have_test_tag(:test_module_id, test_module.id.to_s) + expect(subject).to have_test_tag(:module, module_name) + end + + context "when there is an active test suite" do + let(:test_suite) do + recorder.start_test_suite(test_suite_name) + end + + before do + test_suite + end + + it "connects the test span to the test suite" do + expect(subject).to have_test_tag(:test_suite_id, test_suite.id.to_s) + expect(subject).to have_test_tag(:suite, test_suite_name) + end end end end end - end - context "when given a block" do - before do - recorder.trace_test( - test_name, - test_suite_name, - service: test_service, - tags: tags - ) do |test_span| - test_span.set_metric("my.metric", 42) + context "when given a block" do + before do + recorder.trace_test( + test_name, + test_suite_name, + service: test_service, + tags: tags + ) do |test_span| + test_span.set_metric("my.metric", 42) + end + end + subject { span } + + it "traces and finishes a test" do + expect(subject).to have_test_tag(:name, test_name) + expect(subject.service).to eq(test_service) + expect(subject.name).to eq(test_name) + expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST) + end + + it "sets the provided tags correctly" do + expect(subject).to have_test_tag("test.framework", "my-framework") + expect(subject).to have_test_tag("my.tag", "my_value") + end + + it "sets the suite name in tags" do + expect(subject).to have_test_tag(:suite, test_suite_name) + end + + it_behaves_like "span with environment tags" + it_behaves_like "span with default tags" + it_behaves_like "span with runtime tags" + it_behaves_like "trace with ciapp-test origin" do + let(:trace_under_test) do + trace + end end end - subject { span } + end + + describe "#start_test_session" do + let(:service) { "my-service" } + let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } - it "traces and finishes a test" do - expect(subject).to have_test_tag(:name, test_name) - expect(subject.service).to eq(test_service) - expect(subject.name).to eq(test_name) - expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST) + subject { recorder.start_test_session(service: service, tags: tags) } + + it "returns a new CI test_session span" do + expect(subject).to be_kind_of(Datadog::CI::TestSession) + expect(subject.name).to eq(test_command) + expect(subject.service).to eq(service) + expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST_SESSION) + end + + it "sets the test session id" do + expect(subject).to have_test_tag(:test_session_id, subject.id.to_s) end it "sets the provided tags correctly" do @@ -338,351 +376,355 @@ expect(subject).to have_test_tag("my.tag", "my_value") end - it "sets the suite name in tags" do - expect(subject).to have_test_tag(:suite, test_suite_name) - end - it_behaves_like "span with environment tags" it_behaves_like "span with default tags" it_behaves_like "span with runtime tags" it_behaves_like "trace with ciapp-test origin" do let(:trace_under_test) do + subject.finish + trace end end end - end - - describe "#start_test_session" do - let(:service) { "my-service" } - let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } - subject { recorder.start_test_session(service: service, tags: tags) } + describe "#start_test_module" do + let(:module_name) { "my-module" } + let(:service) { "my-service" } + let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } - it "returns a new CI test_session span" do - expect(subject).to be_kind_of(Datadog::CI::TestSession) - expect(subject.name).to eq(test_command) - expect(subject.service).to eq(service) - expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST_SESSION) - end + subject { recorder.start_test_module(module_name, service: service, tags: tags) } - it "sets the test session id" do - expect(subject).to have_test_tag(:test_session_id, subject.id.to_s) - end + context "when there is no active test session" do + it "returns a new CI test_module span" do + expect(subject).to be_kind_of(Datadog::CI::TestModule) + expect(subject.name).to eq(module_name) + expect(subject.service).to eq(service) + expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST_MODULE) + end - it "sets the provided tags correctly" do - expect(subject).to have_test_tag("test.framework", "my-framework") - expect(subject).to have_test_tag("my.tag", "my_value") - end + it "sets the test module id" do + expect(subject).to have_test_tag(:test_module_id, subject.id.to_s) + end - it_behaves_like "span with environment tags" - it_behaves_like "span with default tags" - it_behaves_like "span with runtime tags" - it_behaves_like "trace with ciapp-test origin" do - let(:trace_under_test) do - subject.finish + it "sets the test module tag" do + expect(subject).to have_test_tag(:module, module_name) + end - trace - end - end - end + it "doesn't connect the test module span to the test session" do + expect(subject).not_to have_test_tag(:test_session_id) + end - describe "#start_test_module" do - let(:module_name) { "my-module" } - let(:service) { "my-service" } - let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } + it "sets the provided tags correctly" do + expect(subject).to have_test_tag("test.framework", "my-framework") + expect(subject).to have_test_tag("my.tag", "my_value") + end - subject { recorder.start_test_module(module_name, service: service, tags: tags) } + it_behaves_like "span with environment tags" + it_behaves_like "span with default tags" + it_behaves_like "span with runtime tags" + it_behaves_like "trace with ciapp-test origin" do + let(:trace_under_test) do + subject.finish - context "when there is no active test session" do - it "returns a new CI test_module span" do - expect(subject).to be_kind_of(Datadog::CI::TestModule) - expect(subject.name).to eq(module_name) - expect(subject.service).to eq(service) - expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST_MODULE) + trace + end + end end - it "sets the test module id" do - expect(subject).to have_test_tag(:test_module_id, subject.id.to_s) - end + context "when there is an active test session" do + let(:service) { nil } + let(:session_service) { "session_service" } + let(:session_tags) { {"test.framework_version" => "1.0", "my.session.tag" => "my_session_value"} } + let(:test_session) { recorder.start_test_session(service: session_service, tags: session_tags) } - it "sets the test module tag" do - expect(subject).to have_test_tag(:module, module_name) - end + before do + test_session + end - it "doesn't connect the test module span to the test session" do - expect(subject).not_to have_test_tag(:test_session_id) - end + it "returns a new CI module span using service from the test session" do + expect(subject).to be_kind_of(Datadog::CI::TestModule) + expect(subject.name).to eq(module_name) + expect(subject.service).to eq(session_service) + end - it "sets the provided tags correctly" do - expect(subject).to have_test_tag("test.framework", "my-framework") - expect(subject).to have_test_tag("my.tag", "my_value") - end + it "sets the provided tags correctly while inheriting some tags from the session" do + expect(subject).to have_test_tag("test.framework", "my-framework") + expect(subject).to have_test_tag("test.framework_version", "1.0") + expect(subject).to have_test_tag("my.tag", "my_value") + expect(subject).not_to have_test_tag("my.session.tag") + end - it_behaves_like "span with environment tags" - it_behaves_like "span with default tags" - it_behaves_like "span with runtime tags" - it_behaves_like "trace with ciapp-test origin" do - let(:trace_under_test) do - subject.finish + it "connects the test module span to the test session" do + expect(subject).to have_test_tag(:test_session_id, test_session.id.to_s) + end - trace + it "does not start a new trace" do + expect(subject.tracer_span.trace_id).to eq(test_session.tracer_span.trace_id) end end end - context "when there is an active test session" do - let(:service) { nil } + describe "start_test_suite" do + let(:module_name) { "my-module" } let(:session_service) { "session_service" } let(:session_tags) { {"test.framework_version" => "1.0", "my.session.tag" => "my_session_value"} } + let(:test_session) { recorder.start_test_session(service: session_service, tags: session_tags) } + let(:test_module) { recorder.start_test_module(module_name) } before do test_session + test_module end - it "returns a new CI module span using service from the test session" do - expect(subject).to be_kind_of(Datadog::CI::TestModule) - expect(subject.name).to eq(module_name) - expect(subject.service).to eq(session_service) - end + context "when test suite with given name is not started yet" do + let(:suite_name) { "my-suite" } + let(:tags) { {"my.tag" => "my_value"} } - it "sets the provided tags correctly while inheriting some tags from the session" do - expect(subject).to have_test_tag("test.framework", "my-framework") - expect(subject).to have_test_tag("test.framework_version", "1.0") - expect(subject).to have_test_tag("my.tag", "my_value") - expect(subject).not_to have_test_tag("my.session.tag") - end + subject { recorder.start_test_suite(suite_name, tags: tags) } - it "connects the test module span to the test session" do - expect(subject).to have_test_tag(:test_session_id, test_session.id.to_s) - end + it "returns a new CI test_suite span" do + expect(subject).to be_kind_of(Datadog::CI::TestSuite) + expect(subject.name).to eq(suite_name) + expect(subject.service).to eq(session_service) + expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST_SUITE) + end - it "does not start a new trace" do - expect(subject.tracer_span.trace_id).to eq(test_session.tracer_span.trace_id) - end - end - end + it "sets the provided tags correctly while inheriting some tags from the session" do + expect(subject).to have_test_tag("test.framework_version", "1.0") + expect(subject).to have_test_tag("my.tag", "my_value") + expect(subject).not_to have_test_tag("my.session.tag") + end - describe "start_test_suite" do - let(:module_name) { "my-module" } - let(:session_service) { "session_service" } - let(:session_tags) { {"test.framework_version" => "1.0", "my.session.tag" => "my_session_value"} } + it "sets the test suite context" do + expect(subject).to have_test_tag(:test_suite_id, subject.id.to_s) + expect(subject).to have_test_tag(:suite, suite_name) + end - let(:test_session) { recorder.start_test_session(service: session_service, tags: session_tags) } - let(:test_module) { recorder.start_test_module(module_name) } + it "sets test session and test module contexts" do + expect(subject).to have_test_tag(:test_session_id, test_session.id.to_s) + expect(subject).to have_test_tag(:test_module_id, test_module.id.to_s) + expect(subject).to have_test_tag(:module, module_name) + end - before do - test_session - test_module - end + it_behaves_like "span with environment tags" + it_behaves_like "span with default tags" + it_behaves_like "span with runtime tags" + end - context "when test suite with given name is not started yet" do - let(:suite_name) { "my-suite" } - let(:tags) { {"my.tag" => "my_value"} } + context "when test suite with given name is already started" do + let(:suite_name) { "my-suite" } + let(:tags) { {"my.tag" => "my_value"} } + let(:already_running_test_suite) { recorder.start_test_suite(suite_name, tags: tags) } - subject { recorder.start_test_suite(suite_name, tags: tags) } + before do + already_running_test_suite + end - it "returns a new CI test_suite span" do - expect(subject).to be_kind_of(Datadog::CI::TestSuite) - expect(subject.name).to eq(suite_name) - expect(subject.service).to eq(session_service) - expect(subject.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST_SUITE) - end + subject { recorder.start_test_suite(suite_name) } - it "sets the provided tags correctly while inheriting some tags from the session" do - expect(subject).to have_test_tag("test.framework_version", "1.0") - expect(subject).to have_test_tag("my.tag", "my_value") - expect(subject).not_to have_test_tag("my.session.tag") + it "returns the already running test suite" do + expect(subject.id).to eq(already_running_test_suite.id) + expect(subject).to have_test_tag("my.tag", "my_value") + end end + end - it "sets the test suite context" do - expect(subject).to have_test_tag(:test_suite_id, subject.id.to_s) - expect(subject).to have_test_tag(:suite, suite_name) + describe "#active_test_session" do + subject { recorder.active_test_session } + context "when there is no active test session" do + it { is_expected.to be_nil } end - it "sets test session and test module contexts" do - expect(subject).to have_test_tag(:test_session_id, test_session.id.to_s) - expect(subject).to have_test_tag(:test_module_id, test_module.id.to_s) - expect(subject).to have_test_tag(:module, module_name) - end + context "when test session is started" do + let(:test_session) { recorder.start_test_session } + before do + test_session + end - it_behaves_like "span with environment tags" - it_behaves_like "span with default tags" - it_behaves_like "span with runtime tags" + it "returns the active test session" do + expect(subject).to be(test_session) + end + end end - context "when test suite with given name is already started" do - let(:suite_name) { "my-suite" } - let(:tags) { {"my.tag" => "my_value"} } - let(:already_running_test_suite) { recorder.start_test_suite(suite_name, tags: tags) } - - before do - already_running_test_suite + describe "#active_test_module" do + subject { recorder.active_test_module } + context "when there is no active test module" do + it { is_expected.to be_nil } end - subject { recorder.start_test_suite(suite_name) } + context "when test module is started" do + let(:test_module) { recorder.start_test_module("my module") } + before do + test_module + end - it "returns the already running test suite" do - expect(subject.id).to eq(already_running_test_suite.id) - expect(subject).to have_test_tag("my.tag", "my_value") + it "returns the active test module" do + expect(subject).to be(test_module) + end end end - end - describe "#active_test_session" do - subject { recorder.active_test_session } - context "when there is no active test session" do - it { is_expected.to be_nil } - end + describe "#active_test" do + subject { recorder.active_test } - context "when test session is started" do - let(:test_session) { recorder.start_test_session } - before do - test_session + context "when there is no active test" do + it { is_expected.to be_nil } end - it "returns the active test session" do - expect(subject).to be(test_session) - end - end - end + context "when test is started" do + let(:ci_test) { recorder.trace_test("my test", "my suite") } - describe "#active_test_module" do - subject { recorder.active_test_module } - context "when there is no active test module" do - it { is_expected.to be_nil } - end - - context "when test module is started" do - let(:test_module) { recorder.start_test_module("my module") } - before do - test_module - end + before do + ci_test + end - it "returns the active test module" do - expect(subject).to be(test_module) + it "returns the active test" do + expect(subject).to be(ci_test) + end end end - end - describe "#active_test" do - subject { recorder.active_test } + describe "#active_span" do + subject { recorder.active_span } - context "when there is no active test" do - it { is_expected.to be_nil } - end + context "when there is no active span" do + it { is_expected.to be_nil } + end - context "when test is started" do - let(:ci_test) { recorder.trace_test("my test", "my suite") } + context "when span is started" do + let(:ci_span) { recorder.trace("my test step", type: "step") } - before do - ci_test - end + before do + ci_span + end - it "returns the active test" do - expect(subject).to be(ci_test) + it "returns a wrapper around the active tracer span" do + expect(subject).to be_kind_of(Datadog::CI::Span) + expect(subject.tracer_span.name).to eq("my test step") + end end end - end - - describe "#active_span" do - subject { recorder.active_span } - context "when there is no active span" do - it { is_expected.to be_nil } - end + describe "#deactivate_test" do + subject { recorder.deactivate_test } - context "when span is started" do - let(:ci_span) { recorder.trace("my test step", type: "step") } + context "when there is no active test" do + let(:ci_test) { Datadog::CI::Test.new(double("tracer span")) } - before do - ci_span + it { is_expected.to be_nil } end - it "returns a wrapper around the active tracer span" do - expect(subject).to be_kind_of(Datadog::CI::Span) - expect(subject.tracer_span.name).to eq("my test step") + context "when deactivating the currently active test" do + let(:ci_test) { recorder.trace_test("my test", "my suite") } + + it "deactivates the test" do + subject + + expect(recorder.active_test).to be_nil + end end end - end - describe "#deactivate_test" do - subject { recorder.deactivate_test } + describe "#deactivate_test_session" do + subject { recorder.deactivate_test_session } - context "when there is no active test" do - let(:ci_test) { Datadog::CI::Test.new(double("tracer span")) } - - it { is_expected.to be_nil } - end + context "when there is no active test session" do + it { is_expected.to be_nil } + end - context "when deactivating the currently active test" do - let(:ci_test) { recorder.trace_test("my test", "my suite") } + context "when deactivating the currently active test session" do + before do + recorder.start_test_session + end - it "deactivates the test" do - subject + it "deactivates the test session" do + subject - expect(recorder.active_test).to be_nil + expect(recorder.active_test_session).to be_nil + end end end - end - - describe "#deactivate_test_session" do - subject { recorder.deactivate_test_session } - context "when there is no active test session" do - it { is_expected.to be_nil } - end + describe "#deactivate_test_module" do + subject { recorder.deactivate_test_module } - context "when deactivating the currently active test session" do - before do - recorder.start_test_session + context "when there is no active test module" do + it { is_expected.to be_nil } end - it "deactivates the test session" do - subject + context "when deactivating the currently active test module" do + before do + recorder.start_test_module("my module") + end + + it "deactivates the test module" do + subject - expect(recorder.active_test_session).to be_nil + expect(recorder.active_test_module).to be_nil + end end end - end - describe "#deactivate_test_module" do - subject { recorder.deactivate_test_module } + describe "#deactivate_test_suite" do + subject { recorder.deactivate_test_suite("my suite") } - context "when there is no active test module" do - it { is_expected.to be_nil } - end - - context "when deactivating the currently active test module" do - before do - recorder.start_test_module("my module") + context "when there is no active test suite" do + it { is_expected.to be_nil } end - it "deactivates the test module" do - subject + context "when deactivating the currently active test suite" do + before do + recorder.start_test_suite("my suite") + end + + it "deactivates the test suite" do + subject - expect(recorder.active_test_module).to be_nil + expect(recorder.active_test_suite("my suite")).to be_nil + end end end end - describe "#deactivate_test_suite" do - subject { recorder.deactivate_test_suite("my suite") } - - context "when there is no active test suite" do - it { is_expected.to be_nil } + context "with ITR" do + include_context "CI mode activated" do + let(:itr_enabled) { true } end - context "when deactivating the currently active test suite" do - before do - recorder.start_test_suite("my suite") - end - - it "deactivates the test suite" do - subject + before do + settings_http_response = double( + "http-response", + ok?: true, + payload: { + "data" => { + "attributes" => { + "itr_enabled" => true, + "tests_skipping" => true, + "code_coverage" => true + } + } + }.to_json + ) + allow_any_instance_of(Datadog::CI::Transport::RemoteSettingsApi).to receive(:fetch_library_settings).and_return( + Datadog::CI::Transport::RemoteSettingsApi::Response.new(settings_http_response) + ) + end + + describe "#start_test_session" do + let(:service) { "my-service" } + let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} } + + subject { recorder.start_test_session(service: service, tags: tags) } + + it "returns a new CI test_session span with ITR tags" do + expect(subject).to be_kind_of(Datadog::CI::TestSession) + expect(subject.service).to eq(service) - expect(recorder.active_test_suite("my suite")).to be_nil + expect(subject.skipping_tests?).to be true + expect(subject.code_coverage?).to be true end end end diff --git a/spec/support/contexts/ci_mode.rb b/spec/support/contexts/ci_mode.rb index 6030a278..17da1f1b 100644 --- a/spec/support/contexts/ci_mode.rb +++ b/spec/support/contexts/ci_mode.rb @@ -13,6 +13,7 @@ let(:ci_enabled) { true } let(:force_test_level_visibility) { false } + let(:itr_enabled) { false } let(:recorder) { Datadog.send(:components).ci_recorder } @@ -26,6 +27,7 @@ Datadog.configure do |c| c.ci.enabled = ci_enabled c.ci.force_test_level_visibility = force_test_level_visibility + c.ci.itr_enabled = itr_enabled unless integration_name == :no_instrument c.ci.instrument integration_name, integration_options end From 46514723a8baafe4fdaec99cbe85bff182469fd4 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 5 Mar 2024 13:59:02 +0100 Subject: [PATCH 17/20] debug logs for ITR configs and change readme for webmock and vcr --- README.md | 4 ++-- lib/datadog/ci/itr/runner.rb | 6 ++++++ lib/datadog/ci/transport/remote_settings_api.rb | 15 ++++++++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3613ef0f..2f414437 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,7 @@ Webmock accordingly. ```ruby # when using agentless mode # note to use the correct datadog site (e.g. datadoghq.eu, etc) -WebMock.disable_net_connect!(:allow => "citestcycle-intake.datadoghq.com") +WebMock.disable_net_connect!(:allow => /datadoghq.com/) # when using agent WebMock.disable_net_connect!(:allow_localhost => true) @@ -199,7 +199,7 @@ VCR.configure do |config| # when using agentless mode # note to use the correct datadog site (e.g. datadoghq.eu, etc) - config.ignore_hosts "citestcycle-intake.datadoghq.com" + config.ignore_hosts "citestcycle-intake.datadoghq.com", "api.datadoghq.com" end ``` diff --git a/lib/datadog/ci/itr/runner.rb b/lib/datadog/ci/itr/runner.rb index 698719b5..f78d8146 100644 --- a/lib/datadog/ci/itr/runner.rb +++ b/lib/datadog/ci/itr/runner.rb @@ -16,9 +16,13 @@ def initialize( @enabled = enabled @test_skipping_enabled = false @code_coverage_enabled = false + + Datadog.logger.debug("ITR Runner initialized with enabled: #{@enabled}") end def configure(remote_configuration, test_session) + Datadog.logger.debug("Configuring ITR Runner with remote configuration: #{remote_configuration}") + @enabled = convert_to_bool( remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY, false) ) @@ -36,6 +40,8 @@ def configure(remote_configuration, test_session) # we skip tests, not suites test_session.set_tag(Ext::Test::TAG_ITR_TEST_SKIPPING_TYPE, Ext::Test::ITR_TEST_SKIPPING_MODE) + + Datadog.logger.debug("Configured ITR Runner with enabled: #{@enabled}, skipping_tests: #{@test_skipping_enabled}, code_coverage: #{@code_coverage_enabled}") end def enabled? diff --git a/lib/datadog/ci/transport/remote_settings_api.rb b/lib/datadog/ci/transport/remote_settings_api.rb index a8c0432b..9e6575a7 100644 --- a/lib/datadog/ci/transport/remote_settings_api.rb +++ b/lib/datadog/ci/transport/remote_settings_api.rb @@ -55,12 +55,17 @@ def fetch_library_settings(test_session) api = @api return Response.new(nil) unless api - Response.new( - api.api_request( - path: Ext::Transport::DD_API_SETTINGS_PATH, - payload: payload(test_session) - ) + request_payload = payload(test_session) + Datadog.logger.debug("Fetching library settings with request: #{request_payload}") + + http_response = api.api_request( + path: Ext::Transport::DD_API_SETTINGS_PATH, + payload: request_payload ) + + Datadog.logger.debug("Library settings response: #{http_response}") + + Response.new(http_response) end private From 86932a81fe306b0b76de4c63b2e98c7c59530a4d Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 6 Mar 2024 11:09:29 +0100 Subject: [PATCH 18/20] add test_level attribute to remote configuration request --- lib/datadog/ci/ext/test.rb | 2 ++ lib/datadog/ci/transport/remote_settings_api.rb | 1 + spec/datadog/ci/transport/remote_settings_api_spec.rb | 1 + 3 files changed, 4 insertions(+) diff --git a/lib/datadog/ci/ext/test.rb b/lib/datadog/ci/ext/test.rb index c3f33922..7c548203 100644 --- a/lib/datadog/ci/ext/test.rb +++ b/lib/datadog/ci/ext/test.rb @@ -50,6 +50,8 @@ module Test TAG_SPAN_KIND = "span.kind" SPAN_KIND_TEST = "test" + # could be either "test" or "suite" depending on whether we skip individual tests or whole suites + # we use test skipping for Ruby ITR_TEST_SKIPPING_MODE = "test" # test status as recognized by Datadog diff --git a/lib/datadog/ci/transport/remote_settings_api.rb b/lib/datadog/ci/transport/remote_settings_api.rb index 9e6575a7..9e99e39c 100644 --- a/lib/datadog/ci/transport/remote_settings_api.rb +++ b/lib/datadog/ci/transport/remote_settings_api.rb @@ -81,6 +81,7 @@ def payload(test_session) "repository_url" => test_session.git_repository_url, "branch" => test_session.git_branch, "sha" => test_session.git_commit_sha, + "test_level" => Ext::Test::ITR_TEST_SKIPPING_MODE, "configurations" => { "os.platform" => test_session.os_platform, "os.arch" => test_session.os_architecture, diff --git a/spec/datadog/ci/transport/remote_settings_api_spec.rb b/spec/datadog/ci/transport/remote_settings_api_spec.rb index 651a47e9..faf31eaf 100644 --- a/spec/datadog/ci/transport/remote_settings_api_spec.rb +++ b/spec/datadog/ci/transport/remote_settings_api_spec.rb @@ -39,6 +39,7 @@ attributes = data["attributes"] expect(attributes["service"]).to eq(service) expect(attributes["env"]).to eq(dd_env) + expect(attributes["test_level"]).to eq("test") expect(attributes["repository_url"]).to eq("repository_url") expect(attributes["branch"]).to eq("branch") expect(attributes["sha"]).to eq("commit_sha") From 309d07aeaf7fa817bd161c38e3abe19e40a1d438 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 6 Mar 2024 11:30:36 +0100 Subject: [PATCH 19/20] Debug log http client response for all requests --- lib/datadog/ci/test_visibility/transport.rb | 4 ---- lib/datadog/ci/transport/http.rb | 8 +++++++- lib/datadog/ci/transport/remote_settings_api.rb | 2 -- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/datadog/ci/test_visibility/transport.rb b/lib/datadog/ci/test_visibility/transport.rb index afc2aa77..45f79a17 100644 --- a/lib/datadog/ci/test_visibility/transport.rb +++ b/lib/datadog/ci/test_visibility/transport.rb @@ -55,10 +55,6 @@ def send_traces(traces) response = send_payload(encoded_payload) - Datadog.logger.debug do - "Received server response: #{response.inspect}" - end - responses << response end diff --git a/lib/datadog/ci/transport/http.rb b/lib/datadog/ci/transport/http.rb index 87bdffc8..5477669d 100644 --- a/lib/datadog/ci/transport/http.rb +++ b/lib/datadog/ci/transport/http.rb @@ -40,11 +40,17 @@ def request(path:, payload:, headers:, verb: "post") "compression_enabled=#{compress}; path=#{path}; payload_size=#{payload.size}" end - ResponseDecorator.new( + response = ResponseDecorator.new( adapter.call( build_env(path: path, payload: payload, headers: headers, verb: verb) ) ) + + Datadog.logger.debug do + "Received server response: #{response.inspect}" + end + + response end private diff --git a/lib/datadog/ci/transport/remote_settings_api.rb b/lib/datadog/ci/transport/remote_settings_api.rb index 9e99e39c..634339ca 100644 --- a/lib/datadog/ci/transport/remote_settings_api.rb +++ b/lib/datadog/ci/transport/remote_settings_api.rb @@ -63,8 +63,6 @@ def fetch_library_settings(test_session) payload: request_payload ) - Datadog.logger.debug("Library settings response: #{http_response}") - Response.new(http_response) end From 06f6752da1460a5ffb5e76fdf5b54397f74dae4b Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 6 Mar 2024 12:18:03 +0100 Subject: [PATCH 20/20] fix settings api path --- lib/datadog/ci/ext/transport.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datadog/ci/ext/transport.rb b/lib/datadog/ci/ext/transport.rb index c6507729..3658abf0 100644 --- a/lib/datadog/ci/ext/transport.rb +++ b/lib/datadog/ci/ext/transport.rb @@ -24,7 +24,7 @@ module Transport TEST_VISIBILITY_INTAKE_PATH = "/api/v2/citestcycle" DD_API_HOST_PREFIX = "api" - DD_API_SETTINGS_PATH = "/api/v2/ci/libraries/tests/services/setting" + DD_API_SETTINGS_PATH = "/api/v2/libraries/tests/services/setting" DD_API_SETTINGS_TYPE = "ci_app_test_service_libraries_settings" DD_API_SETTINGS_RESPONSE_DIG_KEYS = %w[data attributes].freeze DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY = "itr_enabled"