diff --git a/lib/datadog/ci/git/local_repository.rb b/lib/datadog/ci/git/local_repository.rb index 4c0ac47e..e1a4803b 100644 --- a/lib/datadog/ci/git/local_repository.rb +++ b/lib/datadog/ci/git/local_repository.rb @@ -3,12 +3,25 @@ require "open3" require "pathname" +require_relative "../ext/telemetry" +require_relative "telemetry" require_relative "user" module Datadog module CI module Git module LocalRepository + class GitCommandExecutionError < StandardError + attr_reader :output, :command, :status + def initialize(message, output:, command:, status:) + super(message) + + @output = output + @command = command + @status = status + end + end + COMMAND_RETRY_COUNT = 3 def self.root @@ -48,9 +61,18 @@ def self.current_folder_name end def self.git_repository_url - exec_git_command("git ls-remote --get-url") + Telemetry.git_command(Ext::Telemetry::Command::GET_REPOSITORY) + res = nil + + duration_ms = Core::Utils::Time.measure(:float_millisecond) do + res = exec_git_command("git ls-remote --get-url") + end + + Telemetry.git_command_ms(Ext::Telemetry::Command::GET_REPOSITORY, duration_ms) + res rescue => e log_failure(e, "git repository url") + telemetry_track_error(e, Ext::Telemetry::Command::GET_REPOSITORY) nil end @@ -69,9 +91,18 @@ def self.git_commit_sha end def self.git_branch - exec_git_command("git rev-parse --abbrev-ref HEAD") + Telemetry.git_command(Ext::Telemetry::Command::GET_BRANCH) + res = nil + + duration_ms = Core::Utils::Time.measure(:float_millisecond) do + res = exec_git_command("git rev-parse --abbrev-ref HEAD") + end + + Telemetry.git_command_ms(Ext::Telemetry::Command::GET_BRANCH, duration_ms) + res rescue => e log_failure(e, "git branch") + telemetry_track_error(e, Ext::Telemetry::Command::GET_BRANCH) nil end @@ -116,29 +147,49 @@ def self.git_commit_users # returns maximum of 1000 latest commits in the last month def self.git_commits - output = exec_git_command("git log --format=%H -n 1000 --since=\"1 month ago\"") + Telemetry.git_command(Ext::Telemetry::Command::GET_LOCAL_COMMITS) + + output = nil + duration_ms = Core::Utils::Time.measure(:float_millisecond) do + output = exec_git_command("git log --format=%H -n 1000 --since=\"1 month ago\"") + end + + Telemetry.git_command_ms(Ext::Telemetry::Command::GET_LOCAL_COMMITS, duration_ms) + return [] if output.nil? + # @type var output: String output.split("\n") rescue => e log_failure(e, "git commits") + telemetry_track_error(e, Ext::Telemetry::Command::GET_LOCAL_COMMITS) [] end def self.git_commits_rev_list(included_commits:, excluded_commits:) + Telemetry.git_command(Ext::Telemetry::Command::GET_OBJECTS) included_commits = filter_invalid_commits(included_commits).join(" ") excluded_commits = filter_invalid_commits(excluded_commits).map! { |sha| "^#{sha}" }.join(" ") - exec_git_command( - "git rev-list " \ - "--objects " \ - "--no-object-names " \ - "--filter=blob:none " \ - "--since=\"1 month ago\" " \ - "#{excluded_commits} #{included_commits}" - ) + res = nil + + duration_ms = Core::Utils::Time.measure(:float_millisecond) do + res = exec_git_command( + "git rev-list " \ + "--objects " \ + "--no-object-names " \ + "--filter=blob:none " \ + "--since=\"1 month ago\" " \ + "#{excluded_commits} #{included_commits}" + ) + end + + Telemetry.git_command_ms(Ext::Telemetry::Command::GET_OBJECTS, duration_ms) + + res rescue => e log_failure(e, "git commits rev list") + telemetry_track_error(e, Ext::Telemetry::Command::GET_OBJECTS) nil end @@ -150,35 +201,59 @@ def self.git_generate_packfiles(included_commits:, excluded_commits:, path:) basename = SecureRandom.hex(4) - exec_git_command( - "git pack-objects --compression=9 --max-pack-size=3m #{path}/#{basename}", - stdin: commit_tree - ) + Telemetry.git_command(Ext::Telemetry::Command::PACK_OBJECTS) + + duration_ms = Core::Utils::Time.measure(:float_millisecond) do + exec_git_command( + "git pack-objects --compression=9 --max-pack-size=3m #{path}/#{basename}", + stdin: commit_tree + ) + end + Telemetry.git_command_ms(Ext::Telemetry::Command::PACK_OBJECTS, duration_ms) basename rescue => e log_failure(e, "git generate packfiles") + telemetry_track_error(e, Ext::Telemetry::Command::PACK_OBJECTS) nil end def self.git_shallow_clone? - exec_git_command("git rev-parse --is-shallow-repository") == "true" + Telemetry.git_command(Ext::Telemetry::Command::CHECK_SHALLOW) + res = false + + duration_ms = Core::Utils::Time.measure(:float_millisecond) do + res = exec_git_command("git rev-parse --is-shallow-repository") == "true" + end + Telemetry.git_command_ms(Ext::Telemetry::Command::CHECK_SHALLOW, duration_ms) + + res rescue => e log_failure(e, "git shallow clone") + telemetry_track_error(e, Ext::Telemetry::Command::CHECK_SHALLOW) false end def self.git_unshallow - exec_git_command( - "git fetch " \ - "--shallow-since=\"1 month ago\" " \ - "--update-shallow " \ - "--filter=\"blob:none\" " \ - "--recurse-submodules=no " \ - "$(git config --default origin --get clone.defaultRemoteName) $(git rev-parse HEAD)" - ) + Telemetry.git_command(Ext::Telemetry::Command::UNSHALLOW) + res = nil + + duration_ms = Core::Utils::Time.measure(:float_millisecond) do + res = exec_git_command( + "git fetch " \ + "--shallow-since=\"1 month ago\" " \ + "--update-shallow " \ + "--filter=\"blob:none\" " \ + "--recurse-submodules=no " \ + "$(git config --default origin --get clone.defaultRemoteName) $(git rev-parse HEAD)" + ) + end + Telemetry.git_command_ms(Ext::Telemetry::Command::UNSHALLOW, duration_ms) + + res rescue => e log_failure(e, "git unshallow") + telemetry_track_error(e, Ext::Telemetry::Command::UNSHALLOW) nil end @@ -209,7 +284,12 @@ def exec_git_command(cmd, stdin: nil) end if status.nil? || !status.success? - raise "Failed to run git command [#{cmd}] with input [#{stdin}] and output [#{out}]" + raise GitCommandExecutionError.new( + "Failed to run git command [#{cmd}] with input [#{stdin}] and output [#{out}]", + output: out, + command: cmd, + status: status + ) end # Sometimes Encoding.default_external is somehow set to US-ASCII which breaks @@ -231,6 +311,17 @@ def log_failure(e, action) "Unable to perform #{action}: #{e.class.name} #{e.message} at #{Array(e.backtrace).first}" ) end + + def telemetry_track_error(e, command) + case e + when Errno::ENOENT + Telemetry.git_command_errors(command, executable_missing: true) + when GitCommandExecutionError + Telemetry.git_command_errors(command, exit_code: e.status&.to_i) + else + Telemetry.git_command_errors(command, exit_code: -9000) + end + end end end end diff --git a/lib/datadog/ci/git/telemetry.rb b/lib/datadog/ci/git/telemetry.rb new file mode 100644 index 00000000..c779f64f --- /dev/null +++ b/lib/datadog/ci/git/telemetry.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Datadog + module CI + module Git + module Telemetry + def self.git_command(command) + Utils::Telemetry.inc(Ext::Telemetry::METRIC_GIT_COMMAND, 1, tags_for_command(command)) + end + + def self.git_command_errors(command, exit_code: nil, executable_missing: false) + tags = tags_for_command(command) + + exit_code_tag_value = exit_code_for(exit_code: exit_code, executable_missing: executable_missing) + tags[Ext::Telemetry::TAG_EXIT_CODE] = exit_code_tag_value if exit_code_tag_value + + Utils::Telemetry.inc(Ext::Telemetry::METRIC_GIT_COMMAND_ERRORS, 1, tags) + end + + def self.git_command_ms(command, duration_ms) + Utils::Telemetry.distribution(Ext::Telemetry::METRIC_GIT_COMMAND_MS, duration_ms, tags_for_command(command)) + end + + def self.tags_for_command(command) + {Ext::Telemetry::TAG_COMMAND => command} + end + + def self.exit_code_for(exit_code: nil, executable_missing: false) + return Ext::Telemetry::ExitCode::MISSING if executable_missing + return exit_code.to_s if exit_code + + nil + end + end + end + end +end diff --git a/lib/datadog/ci/test_visibility/component.rb b/lib/datadog/ci/test_visibility/component.rb index 1a5ae494..d070c1d4 100644 --- a/lib/datadog/ci/test_visibility/component.rb +++ b/lib/datadog/ci/test_visibility/component.rb @@ -8,7 +8,6 @@ require_relative "../codeowners/parser" require_relative "../contrib/contrib" require_relative "../ext/test" -require_relative "../ext/environment" require_relative "../git/local_repository" require_relative "../worker" @@ -28,7 +27,7 @@ def initialize( codeowners: Codeowners::Parser.new(Git::LocalRepository.root).parse ) @test_suite_level_visibility_enabled = test_suite_level_visibility_enabled - @context = Context.new(Ext::Environment.tags(ENV).freeze) + @context = Context.new @codeowners = codeowners @test_optimisation = test_optimisation @remote_settings_api = remote_settings_api diff --git a/lib/datadog/ci/test_visibility/context.rb b/lib/datadog/ci/test_visibility/context.rb index 0eb2f42c..2405bcf5 100644 --- a/lib/datadog/ci/test_visibility/context.rb +++ b/lib/datadog/ci/test_visibility/context.rb @@ -8,6 +8,7 @@ require_relative "store/local" require_relative "../ext/app_types" +require_relative "../ext/environment" require_relative "../ext/test" require_relative "../span" @@ -23,11 +24,9 @@ module TestVisibility # Its responsibility includes building domain models for test visibility as well. # Internally it uses Datadog::Tracing module to create spans. class Context - def initialize(environment_tags) + def initialize @local_context = Store::Local.new @global_context = Store::Global.new - - @environment_tags = environment_tags end def start_test_session(service: nil, tags: {}) @@ -197,6 +196,8 @@ def build_span(tracer_span, tags) # TAGGING def set_initial_tags(ci_span, tags) + @environment_tags ||= Ext::Environment.tags(ENV).freeze + ci_span.set_default_tags ci_span.set_environment_runtime_tags diff --git a/sig/datadog/ci/git/local_repository.rbs b/sig/datadog/ci/git/local_repository.rbs index 25f4db7e..24a9d68f 100644 --- a/sig/datadog/ci/git/local_repository.rbs +++ b/sig/datadog/ci/git/local_repository.rbs @@ -2,6 +2,14 @@ module Datadog module CI module Git module LocalRepository + class GitCommandExecutionError < StandardError + attr_reader command: String + attr_reader output: String? + attr_reader status: Process::Status? + + def initialize: (String message, output: String?, status: Process::Status?, command: String) -> void + end + COMMAND_RETRY_COUNT: 3 @root: String? @@ -46,6 +54,8 @@ module Datadog def self.exec_git_command: (String ref, ?stdin: String?) -> String? def self.log_failure: (StandardError e, String action) -> void + + def self.telemetry_track_error: (StandardError e, String command) -> void end end end diff --git a/sig/datadog/ci/git/telemetry.rbs b/sig/datadog/ci/git/telemetry.rbs new file mode 100644 index 00000000..633bdfdd --- /dev/null +++ b/sig/datadog/ci/git/telemetry.rbs @@ -0,0 +1,17 @@ +module Datadog + module CI + module Git + module Telemetry + def self.git_command: (String command) -> void + + def self.git_command_errors: (String command, ?exit_code: Integer?, ?executable_missing: bool) -> void + + def self.git_command_ms: (String command, untyped duration_ms) -> void + + def self.tags_for_command: (String command) -> ::Hash[String, String] + + def self.exit_code_for: (?exit_code: Integer?, ?executable_missing: bool) -> String? + end + end + end +end diff --git a/sig/datadog/ci/test_visibility/context.rbs b/sig/datadog/ci/test_visibility/context.rbs index a4b8efb0..bf785a19 100644 --- a/sig/datadog/ci/test_visibility/context.rbs +++ b/sig/datadog/ci/test_visibility/context.rbs @@ -7,7 +7,7 @@ module Datadog @environment_tags: Hash[String, String] - def initialize: (Hash[String, String] environment_tags) -> void + def initialize: () -> 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 5e8af105..8608edac 100644 --- a/spec/datadog/ci/configuration/components_spec.rb +++ b/spec/datadog/ci/configuration/components_spec.rb @@ -107,10 +107,6 @@ context "is enabled" do let(:enabled) { true } - it "collects environment tags" do - expect(Datadog::CI::Ext::Environment).to have_received(:tags).with(ENV) - end - context "when tracing is disabled" do let(:tracing_enabled) { false } @@ -345,10 +341,6 @@ expect(settings.tracing.test_mode) .to_not have_received(:writer_options=) end - - it "does not collect tags" do - expect(Datadog::CI::Ext::Environment).not_to have_received(:tags) - end end end end diff --git a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb index 8387d879..96189abc 100644 --- a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb @@ -66,7 +66,9 @@ steps_file_for_run_path ) - expect(Datadog::CI::Ext::Environment).to receive(:tags).never + # assert that environment tags are collected once per session + expect(Datadog::CI::Ext::Environment).to receive(:tags).once.and_call_original + expect(kernel).to receive(:exit).with(expected_test_run_code) # do not use manual API diff --git a/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb b/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb index 67ede067..165f117f 100644 --- a/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb @@ -94,7 +94,7 @@ def test_foo end it "creates spans for several tests" do - expect(Datadog::CI::Ext::Environment).to receive(:tags).never + expect(Datadog::CI::Ext::Environment).to receive(:tags).once.and_call_original num_tests = 20 diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index f855bb62..fc301ce8 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -125,7 +125,7 @@ def rspec_session_run( end it "creates spans for several examples" do - expect(Datadog::CI::Ext::Environment).to receive(:tags).never + expect(Datadog::CI::Ext::Environment).to receive(:tags).once.and_call_original num_examples = 20 with_new_rspec_environment do diff --git a/spec/datadog/ci/git/local_repository_spec.rb b/spec/datadog/ci/git/local_repository_spec.rb index 08b0af21..622774cf 100644 --- a/spec/datadog/ci/git/local_repository_spec.rb +++ b/spec/datadog/ci/git/local_repository_spec.rb @@ -3,6 +3,8 @@ require_relative "../../../../lib/datadog/ci/git/local_repository" RSpec.describe ::Datadog::CI::Git::LocalRepository do + include_context "Telemetry spy" + let(:environment_variables) { {} } def with_custom_git_environment @@ -103,6 +105,9 @@ def with_custom_git_environment subject { described_class.git_repository_url } it { is_expected.to eq("git@github.com:DataDog/datadog-ci-rb.git") } + + it_behaves_like "emits telemetry metric", :inc, "git.command", 1 + it_behaves_like "emits telemetry metric", :distribution, "git.command_ms" end context "with git folder" do @@ -136,6 +141,9 @@ def with_custom_git_environment end it { is_expected.to eq("master") } + + it_behaves_like "emits telemetry metric", :inc, "git.command", 1 + it_behaves_like "emits telemetry metric", :distribution, "git.command_ms" end describe ".git_tag" do @@ -187,6 +195,9 @@ def with_custom_git_environment it "returns empty array as last commit was more than 1 month ago" do expect(subject).to eq([]) end + + it_behaves_like "emits telemetry metric", :inc, "git.command", 1 + it_behaves_like "emits telemetry metric", :distribution, "git.command_ms" end describe ".git_commits_rev_list" do @@ -200,6 +211,9 @@ def with_custom_git_environment end it { is_expected.to be_nil } + + it_behaves_like "emits telemetry metric", :inc, "git.command", 1 + it_behaves_like "emits telemetry metric", :distribution, "git.command_ms" end end @@ -417,6 +431,49 @@ def with_clone_git_dir end it { is_expected.to eq([]) } + + it_behaves_like "emits telemetry metric", :inc, "git.command_errors", 1 + + it "tags error metric with command" do + subject + + metric = telemetry_metric(:inc, "git.command_errors") + expect(metric.tags).to eq({"command" => "get_local_commits"}) + end + end + + context "returns exit code 1" do + before do + expect(Open3).to receive(:capture2e).and_return(["error", double(success?: false, to_i: 1)]) + end + + it { is_expected.to eq([]) } + + it_behaves_like "emits telemetry metric", :inc, "git.command_errors", 1 + + it "tags error metric with command" do + subject + + metric = telemetry_metric(:inc, "git.command_errors") + expect(metric.tags).to eq({"command" => "get_local_commits", "exit_code" => "1"}) + end + end + + context "git executable is missing" do + before do + expect(Open3).to receive(:capture2e).and_raise(Errno::ENOENT.new("no file or directoru")) + end + + it { is_expected.to eq([]) } + + it_behaves_like "emits telemetry metric", :inc, "git.command_errors", 1 + + it "tags error metric with command" do + subject + + metric = telemetry_metric(:inc, "git.command_errors") + expect(metric.tags).to eq({"command" => "get_local_commits", "exit_code" => "missing"}) + end end end end diff --git a/spec/datadog/ci/git/telemetry_spec.rb b/spec/datadog/ci/git/telemetry_spec.rb new file mode 100644 index 00000000..80c55dee --- /dev/null +++ b/spec/datadog/ci/git/telemetry_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require_relative "../../../../lib/datadog/ci/git/telemetry" + +RSpec.describe Datadog::CI::Git::Telemetry do + describe ".git_command" do + subject(:git_command) { described_class.git_command(command) } + + let(:command) { "git ls-remote --get-url" } + + before do + expect(Datadog::CI::Utils::Telemetry).to receive(:inc) + .with(Datadog::CI::Ext::Telemetry::METRIC_GIT_COMMAND, 1, expected_tags) + end + + let(:expected_tags) do + { + Datadog::CI::Ext::Telemetry::TAG_COMMAND => command + } + end + + it { git_command } + end + + describe ".git_command_errors" do + subject(:git_command_errors) { described_class.git_command_errors(command, exit_code: exit_code, executable_missing: executable_missing) } + + let(:command) { "git ls-remote --get-url" } + before do + expect(Datadog::CI::Utils::Telemetry).to receive(:inc) + .with(Datadog::CI::Ext::Telemetry::METRIC_GIT_COMMAND_ERRORS, 1, expected_tags) + end + + context "when exit code is 1" do + let(:exit_code) { 1 } + let(:executable_missing) { false } + + let(:expected_tags) do + { + Datadog::CI::Ext::Telemetry::TAG_COMMAND => command, + Datadog::CI::Ext::Telemetry::TAG_EXIT_CODE => exit_code.to_s + } + end + + it { git_command_errors } + end + + context "when executable is missing" do + let(:executable_missing) { true } + let(:exit_code) { nil } + + let(:expected_tags) do + { + Datadog::CI::Ext::Telemetry::TAG_COMMAND => command, + Datadog::CI::Ext::Telemetry::TAG_EXIT_CODE => Datadog::CI::Ext::Telemetry::ExitCode::MISSING + } + end + + it { git_command_errors } + end + end + + describe ".git_command_ms" do + subject(:git_command_ms) { described_class.git_command_ms(command, duration_ms) } + + let(:command) { "git ls-remote --get-url" } + let(:duration_ms) { 100 } + + before do + expect(Datadog::CI::Utils::Telemetry).to receive(:distribution) + .with(Datadog::CI::Ext::Telemetry::METRIC_GIT_COMMAND_MS, duration_ms, expected_tags) + end + + let(:expected_tags) do + { + Datadog::CI::Ext::Telemetry::TAG_COMMAND => command + } + end + + it { git_command_ms } + end +end diff --git a/spec/datadog/ci/transport/http_spec.rb b/spec/datadog/ci/transport/http_spec.rb index 2482bf9f..5bf0b272 100644 --- a/spec/datadog/ci/transport/http_spec.rb +++ b/spec/datadog/ci/transport/http_spec.rb @@ -118,7 +118,7 @@ expect(response.code).to eq(200) expect(response.payload).to eq("sample payload") expect(response.request_compressed).to eq(false) - expect(response.request_size).to eq(payload.size) + expect(response.request_size).to eq(payload.bytesize) expect(response.telemetry_error_type).to be_nil end @@ -155,7 +155,7 @@ expect(response.code).to eq(200) expect(response.request_compressed).to eq(true) - expect(response.request_size).to eq(expected_payload.size) + expect(response.request_size).to eq(expected_payload.bytesize) end end