diff --git a/common/lib/dependabot/requirement.rb b/common/lib/dependabot/requirement.rb index 4a161ab0f5..88eff4577a 100644 --- a/common/lib/dependabot/requirement.rb +++ b/common/lib/dependabot/requirement.rb @@ -29,7 +29,7 @@ def constraints end # Returns the highest lower limit among all minimum constraints. - sig { returns(T.nilable(Gem::Version)) } + sig { returns(T.nilable(Dependabot::Version)) } def min_version # Select constraints with minimum operators min_constraints = requirements.select { |op, _| MINIMUM_OPERATORS.include?(op) } diff --git a/updater/lib/dependabot/api_client.rb b/updater/lib/dependabot/api_client.rb index 24b408228b..c0ebe9be9a 100644 --- a/updater/lib/dependabot/api_client.rb +++ b/updater/lib/dependabot/api_client.rb @@ -21,6 +21,8 @@ class ApiError < StandardError; end class ApiClient extend T::Sig + MAX_REQUEST_RETRIES = 3 + sig { params(base_url: String, job_id: T.any(String, Integer), job_token: String).void } def initialize(base_url, job_id, job_token) @base_url = base_url @@ -43,7 +45,7 @@ def create_pull_request(dependency_change, base_commit_sha) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -72,7 +74,7 @@ def update_pull_request(dependency_change, base_commit_sha) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -92,7 +94,7 @@ def close_pull_request(dependency_names, reason) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -119,7 +121,7 @@ def record_update_job_error(error_type:, error_details:) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -154,7 +156,7 @@ def record_update_job_warning(warn_type:, warn_title:, warn_description:) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -180,7 +182,7 @@ def record_update_job_unknown_error(error_type:, error_details:) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -200,7 +202,7 @@ def mark_job_as_processed(base_commit_sha) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -224,7 +226,7 @@ def update_dependency_list(dependencies, dependency_files) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -243,7 +245,7 @@ def record_ecosystem_versions(ecosystem_versions) rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError retry_count ||= 0 retry_count += 1 - raise if retry_count > 3 + raise if retry_count > MAX_REQUEST_RETRIES sleep(rand(3.0..10.0)) retry @@ -274,8 +276,86 @@ def increment_metric(metric, tags:) end end + sig { params(ecosystem: T.nilable(Ecosystem)).void } + def record_ecosystem_meta(ecosystem) + return unless Dependabot::Experiments.enabled?(:enable_record_ecosystem_meta) + + return if ecosystem.nil? + + begin + ::Dependabot::OpenTelemetry.tracer.in_span("record_ecosystem_meta", kind: :internal) do |_span| + api_url = "#{base_url}/update_jobs/#{job_id}/record_ecosystem_meta" + + body = { + data: [ + { + ecosystem: { + name: ecosystem.name, + package_manager: version_manager_json(ecosystem.package_manager), + language: version_manager_json(ecosystem.language) + } + } + ] + } + + retry_count = 0 + + begin + response = http_client.post(api_url, json: body) + raise ApiError, response.body if response.code >= 400 + rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError, ApiError => e + retry_count += 1 + if retry_count <= MAX_REQUEST_RETRIES + sleep(rand(3.0..10.0)) + retry + else + Dependabot.logger.error( + "Failed to record ecosystem meta after #{MAX_REQUEST_RETRIES} retries: #{e.message}" + ) + end + end + end + rescue StandardError => e + Dependabot.logger.error("Failed to record ecosystem meta: #{e.message}") + end + end + private + # Update return type to allow returning a Hash or nil + sig do + params(version_manager: T.nilable(Dependabot::Ecosystem::VersionManager)) + .returns(T.nilable(T::Hash[String, T.untyped])) + end + def version_manager_json(version_manager) + return nil unless version_manager + + { + name: version_manager.name, + raw_version: version_manager.version.to_semver.to_s, + version: version_manager.version.to_s, + requirement: version_manager_requirement_json(version_manager) + } + end + + # Update return type to allow returning a Hash or nil + sig do + params(version_manager: Dependabot::Ecosystem::VersionManager) + .returns(T.nilable(T::Hash[String, T.untyped])) + end + def version_manager_requirement_json(version_manager) + requirement = version_manager.requirement + return nil unless requirement + + { + raw_constraint: requirement.constraints.join(", "), + min_raw_version: requirement.min_version&.to_semver.to_s, + min_version: requirement.min_version&.to_s, + max_raw_version: requirement.max_version&.to_semver.to_s, + max_version: requirement.max_version&.to_s + } + end + sig { returns(String) } attr_reader :base_url diff --git a/updater/lib/dependabot/service.rb b/updater/lib/dependabot/service.rb index c8cb0f2e5d..828fd4944c 100644 --- a/updater/lib/dependabot/service.rb +++ b/updater/lib/dependabot/service.rb @@ -38,7 +38,8 @@ def initialize(client:) def_delegators :client, :mark_job_as_processed, :record_ecosystem_versions, - :increment_metric + :increment_metric, + :record_ecosystem_meta sig { void } def wait_for_calls_to_finish diff --git a/updater/lib/dependabot/updater/group_update_refreshing.rb b/updater/lib/dependabot/updater/group_update_refreshing.rb index 74b1826900..2067b53a59 100644 --- a/updater/lib/dependabot/updater/group_update_refreshing.rb +++ b/updater/lib/dependabot/updater/group_update_refreshing.rb @@ -41,6 +41,9 @@ def upsert_pull_request_with_error_handling(dependency_change, group) end rescue StandardError => e error_handler.handle_job_error(error: e, dependency_group: dependency_snapshot.job_group) + ensure + # record metrics for the ecosystem + service.record_ecosystem_meta(dependency_snapshot.ecosystem) end # Having created the dependency_change, we need to determine the right strategy to apply it to the project: diff --git a/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb b/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb index 91c047ba85..99b4e430b1 100644 --- a/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb @@ -64,6 +64,8 @@ def perform service.create_pull_request(T.must(dependency_change), dependency_snapshot.base_commit_sha) rescue StandardError => e error_handler.handle_job_error(error: e, dependency_group: group) + ensure + service.record_ecosystem_meta(dependency_snapshot.ecosystem) end else Dependabot.logger.info("Nothing to update for Dependency Group: '#{group.name}'") diff --git a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb index 156a48f3b3..1f6945c9b4 100644 --- a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb @@ -100,6 +100,8 @@ def check_and_create_pr_with_error_handling(dependency) ) rescue StandardError => e error_handler.handle_dependency_error(error: e, dependency: dependency) + ensure + service.record_ecosystem_meta(dependency_snapshot.ecosystem) end # rubocop:disable Metrics/AbcSize diff --git a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb index ab94b2fad2..e397871f9e 100644 --- a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb @@ -61,6 +61,9 @@ def perform check_and_update_pull_request(dependencies) rescue StandardError => e error_handler.handle_dependency_error(error: e, dependency: dependencies.last) + ensure + # Record ecosystem metrics for the update job + service.record_ecosystem_meta(dependency_snapshot.ecosystem) end private diff --git a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb index aafa2b2e13..29a13cb2b7 100644 --- a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb @@ -67,6 +67,8 @@ def perform check_and_update_pull_request(dependencies) rescue StandardError => e error_handler.handle_dependency_error(error: e, dependency: dependency) + ensure + service.record_ecosystem_meta(dependency_snapshot.ecosystem) end private diff --git a/updater/lib/dependabot/updater/operations/update_all_versions.rb b/updater/lib/dependabot/updater/operations/update_all_versions.rb index bdcc9a20c4..85f8aa2c42 100644 --- a/updater/lib/dependabot/updater/operations/update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/update_all_versions.rb @@ -93,8 +93,10 @@ def dependencies def check_and_create_pr_with_error_handling(dependency) check_and_create_pull_request(dependency) rescue URI::InvalidURIError => e - error_handler.handle_dependency_error(error: Dependabot::DependencyFileNotResolvable.new(e.message), - dependency: dependency) + error_handler.handle_dependency_error( + error: Dependabot::DependencyFileNotResolvable.new(e.message), + dependency: dependency + ) rescue Dependabot::InconsistentRegistryResponse => e error_handler.log_dependency_error( dependency: dependency, @@ -104,6 +106,8 @@ def check_and_create_pr_with_error_handling(dependency) ) rescue StandardError => e process_dependency_error(e, dependency) + ensure + service.record_ecosystem_meta(dependency_snapshot.ecosystem) end # rubocop:disable Metrics/AbcSize diff --git a/updater/spec/dependabot/api_client_spec.rb b/updater/spec/dependabot/api_client_spec.rb index 36779ba6fc..af0cd8dded 100644 --- a/updater/spec/dependabot/api_client_spec.rb +++ b/updater/spec/dependabot/api_client_spec.rb @@ -77,7 +77,7 @@ before do allow(Dependabot::PullRequestCreator::MessageBuilder).to receive_message_chain(:new, :message).and_return(message) - + allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_record_ecosystem_meta).and_return(true) stub_request(:post, create_pull_request_url) .to_return(status: 204, headers: headers) end @@ -509,4 +509,87 @@ end end end + + describe "record_ecosystem_meta" do + before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_record_ecosystem_meta).and_return(true) + end + + let(:ecosystem) do + Dependabot::Ecosystem.new( + name: "bundler", + package_manager: instance_double( + Dependabot::Ecosystem::VersionManager, + name: "bundler", + version: Dependabot::Version.new("2.1.4"), + requirement: instance_double( + Dependabot::Requirement, + constraints: [">= 2.0"], + min_version: Dependabot::Version.new("2.0.0"), + max_version: Dependabot::Version.new("3.0.0") + ) + ), + language: instance_double( + Dependabot::Ecosystem::VersionManager, + name: "ruby", + version: Dependabot::Version.new("2.7.0"), + requirement: nil + ) + ) + end + let(:record_ecosystem_meta_url) { "http://example.com/update_jobs/1/record_ecosystem_meta" } + + it "hits the correct endpoint" do + client.record_ecosystem_meta(ecosystem) + + expect(WebMock) + .to have_requested(:post, record_ecosystem_meta_url) + .with(headers: { "Authorization" => "token" }) + end + + it "encodes the payload correctly" do + client.record_ecosystem_meta(ecosystem) + + expect(WebMock).to(have_requested(:post, record_ecosystem_meta_url).with do |req| + data = JSON.parse(req.body)["data"][0]["ecosystem"] + + expect(data).not_to be_nil # Ensure data is present + expect(data["name"]).to eq("bundler") + expect(data["package_manager"]).to include( + "name" => "bundler", + "raw_version" => "2.1.4", + "version" => "2.1.4", + "requirement" => { + "max_raw_version" => "3.0.0", + "max_version" => "3.0.0", + "min_raw_version" => "2.0.0", + "min_version" => "2.0.0", + "raw_constraint" => ">= 2.0" + } + ) + expect(data["language"]).to include( + "name" => "ruby", + "version" => "2.7.0" + ) + end) + end + + context "when ecosystem is nil" do + it "does not send a request" do + client.record_ecosystem_meta(nil) + expect(WebMock).not_to have_requested(:post, record_ecosystem_meta_url) + end + end + + context "when feature flag is disabled" do + before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_record_ecosystem_meta).and_return(false) + end + + it "does not send a request" do + client.record_ecosystem_meta(ecosystem) + expect(WebMock).not_to have_requested(:post, record_ecosystem_meta_url) + end + end + end end diff --git a/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb index 15d904b453..ec49f1f43f 100644 --- a/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb @@ -32,7 +32,14 @@ end let(:mock_service) do - instance_double(Dependabot::Service, create_pull_request: nil, update_pull_request: nil, close_pull_request: nil) + instance_double( + Dependabot::Service, + increment_metric: nil, + record_update_job_error: nil, + create_pull_request: nil, + record_update_job_warning: nil, + record_ecosystem_meta: nil + ) end let(:mock_error_handler) { instance_double(Dependabot::Updater::ErrorHandler) } diff --git a/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb index eea4493fe3..a60d5d3d26 100644 --- a/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb @@ -36,7 +36,8 @@ increment_metric: nil, record_update_job_error: nil, create_pull_request: nil, - record_update_job_warning: nil + record_update_job_warning: nil, + record_ecosystem_meta: nil ) end diff --git a/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb index 7ed869447a..a68397ce6c 100644 --- a/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb @@ -27,7 +27,14 @@ end let(:mock_service) do - instance_double(Dependabot::Service, increment_metric: nil) + instance_double( + Dependabot::Service, + increment_metric: nil, + record_update_job_error: nil, + create_pull_request: nil, + record_update_job_warning: nil, + record_ecosystem_meta: nil + ) end let(:job) do diff --git a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb index fbc8aa9bc1..e69e456adf 100644 --- a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb @@ -31,8 +31,16 @@ end let(:mock_service) do - instance_double(Dependabot::Service, create_pull_request: nil, update_pull_request: nil, close_pull_request: nil) + instance_double( + Dependabot::Service, + increment_metric: nil, + record_update_job_error: nil, + create_pull_request: nil, + record_update_job_warning: nil, + record_ecosystem_meta: nil + ) end + let(:mock_error_handler) { instance_double(Dependabot::Updater::ErrorHandler) } let(:job_definition) do @@ -148,6 +156,8 @@ allow(Dependabot::DependencyChangeBuilder) .to receive(:create_from) .and_return(stub_dependency_change) + + allow(mock_service).to receive(:close_pull_request) end after do diff --git a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb index 565ef59006..dbeeed37b0 100644 --- a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb @@ -32,7 +32,13 @@ end let(:mock_service) do - instance_double(Dependabot::Service, create_pull_request: nil, update_pull_request: nil, close_pull_request: nil) + instance_double( + Dependabot::Service, + create_pull_request: nil, + update_pull_request: nil, + close_pull_request: nil, + record_ecosystem_meta: nil + ) end let(:mock_error_handler) { instance_double(Dependabot::Updater::ErrorHandler) } diff --git a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb index b98d18f555..c64ca7e7e2 100644 --- a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb +++ b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb @@ -37,9 +37,10 @@ instance_double( Dependabot::Service, increment_metric: nil, + record_update_job_error: nil, create_pull_request: nil, - close_pull_request: nil, - record_update_job_warning: nil + record_update_job_warning: nil, + record_ecosystem_meta: nil ) end diff --git a/updater/spec/dependabot/updater_spec.rb b/updater/spec/dependabot/updater_spec.rb index d3005d1192..91b4aefc1b 100644 --- a/updater/spec/dependabot/updater_spec.rb +++ b/updater/spec/dependabot/updater_spec.rb @@ -2702,7 +2702,8 @@ def build_service mark_job_as_processed: nil, record_update_job_error: nil, record_update_job_unknown_error: nil, - increment_metric: nil + increment_metric: nil, + record_ecosystem_meta: nil ) allow(api_client).to receive(:is_a?).with(Dependabot::ApiClient).and_return(true)