Skip to content

Commit

Permalink
added sending bundler v1 deprecation warning alert (#10485)
Browse files Browse the repository at this point in the history
  • Loading branch information
kbukum1 authored Sep 4, 2024
1 parent 78fc799 commit 921363f
Show file tree
Hide file tree
Showing 15 changed files with 534 additions and 171 deletions.
42 changes: 40 additions & 2 deletions updater/lib/dependabot/api_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,11 @@ def close_pull_request(dependency_names, reason)
sig { params(error_type: T.any(String, Symbol), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
def record_update_job_error(error_type:, error_details:)
::Dependabot::OpenTelemetry.tracer.in_span("record_update_job_error", kind: :internal) do |_span|
::Dependabot::OpenTelemetry.record_update_job_error(job_id: job_id, error_type: error_type,
error_details: error_details)
::Dependabot::OpenTelemetry.record_update_job_error(
job_id: job_id,
error_type: error_type,
error_details: error_details
)
api_url = "#{base_url}/update_jobs/#{job_id}/record_update_job_error"
body = {
data: {
Expand All @@ -123,6 +126,41 @@ def record_update_job_error(error_type:, error_details:)
end
end

sig do
params(
warn_type: T.any(String, Symbol),
warn_title: String,
warn_description: String
).void
end
def record_update_job_warning(warn_type:, warn_title:, warn_description:)
::Dependabot::OpenTelemetry.tracer.in_span("record_update_job_message", kind: :internal) do |_span|
::Dependabot::OpenTelemetry.record_update_job_warning(
job_id: job_id,
warn_type: warn_type,
warn_title: warn_title,
warn_description: warn_description
)
api_url = "#{base_url}/update_jobs/#{job_id}/record_update_job_warning"
body = {
data: {
"warn-type": warn_type,
"warn-title": warn_title,
"warn-description": warn_description
}
}
response = http_client.post(api_url, json: body)
raise ApiError, response.body if response.code >= 400
rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError
retry_count ||= 0
retry_count += 1
raise if retry_count > 3

sleep(rand(3.0..10.0))
retry
end
end

sig { params(error_type: T.any(Symbol, String), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
def record_update_job_unknown_error(error_type:, error_details:)
error_type = "unknown_error" if error_type.nil?
Expand Down
23 changes: 23 additions & 0 deletions updater/lib/dependabot/opentelemetry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ module OpenTelemetry

module Attributes
JOB_ID = "dependabot.job.id"
WARN_TYPE = "dependabot.job.warn_type"
WARN_TITLE = "dependabot.job.warn_title"
WARN_DESCRIPTION = "dependabot.job.warn_description"
ERROR_TYPE = "dependabot.job.error_type"
ERROR_DETAILS = "dependabot.job.error_details"
METRIC = "dependabot.metric"
Expand Down Expand Up @@ -89,6 +92,26 @@ def self.record_update_job_error(job_id:, error_type:, error_details:)
current_span.add_event(error_type, attributes: attributes)
end

sig do
params(
job_id: T.any(String, Integer),
warn_type: T.any(String, Symbol),
warn_title: String,
warn_description: String
).void
end
def self.record_update_job_warning(job_id:, warn_type:, warn_title:, warn_description:)
current_span = ::OpenTelemetry::Trace.current_span

attributes = {
Attributes::JOB_ID => job_id,
Attributes::WARN_TYPE => warn_type,
Attributes::WARN_TITLE => warn_title,
Attributes::WARN_DESCRIPTION => warn_description
}
current_span.add_event(warn_type, attributes: attributes)
end

sig do
params(
error: StandardError,
Expand Down
15 changes: 15 additions & 0 deletions updater/lib/dependabot/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,21 @@ def record_update_job_error(error_type:, error_details:, dependency: nil)
client.record_update_job_error(error_type: error_type, error_details: error_details)
end

sig do
params(
warn_type: T.any(String, Symbol),
warn_title: String,
warn_description: String
).void
end
def record_update_job_warning(warn_type:, warn_title:, warn_description:)
client.record_update_job_warning(
warn_type: warn_type,
warn_title: warn_title,
warn_description: warn_description
)
end

sig { params(error_type: T.any(String, Symbol), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void }
def record_update_job_unknown_error(error_type:, error_details:)
client.record_update_job_unknown_error(error_type: error_type, error_details: error_details)
Expand Down
5 changes: 5 additions & 0 deletions updater/lib/dependabot/updater/group_update_creation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Updater
module GroupUpdateCreation
extend T::Sig
extend T::Helpers
include PullRequestHelpers

abstract!

Expand Down Expand Up @@ -122,6 +123,10 @@ def compile_all_dependency_changes_for(group)
return nil
end

# Send warning alerts to the API if any warning notices are present.
# Note that only notices with notice.show_alert set to true will be sent.
record_warning_notices(notices) if notices.any?

dependency_change
ensure
cleanup_workspace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module Operations
class CreateSecurityUpdatePullRequest
extend T::Sig
include SecurityUpdateHelpers
include PullRequestHelpers

sig { params(job: Job).returns(T::Boolean) }
def self.applies_to?(job:)
Expand Down Expand Up @@ -180,9 +181,14 @@ def check_and_create_pull_request(dependency)
dependency_files: dependency_snapshot.dependency_files,
updated_dependencies: updated_deps,
change_source: checker.dependency,
# Sending notices to the pr message builder to be used in the PR message if show_in_pr is true
notices: @notices
)

# Send warning alerts to the API if any warning notices are present.
# Note that only notices with notice.show_alert set to true will be sent.
record_warning_notices(notices) if notices.any?

create_pull_request(dependency_change)
rescue Dependabot::AllVersionsIgnored
Dependabot.logger.info("All updates for #{dependency.name} were ignored")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ module Operations
class RefreshSecurityUpdatePullRequest
extend T::Sig
include SecurityUpdateHelpers
include PullRequestHelpers

sig { params(job: Job).returns(T::Boolean) }
def self.applies_to?(job:)
Expand Down Expand Up @@ -158,9 +159,14 @@ def check_and_update_pull_request(dependencies)
dependency_files: dependency_snapshot.dependency_files,
updated_dependencies: updated_deps,
change_source: checker.dependency,
# Sending notices to the pr message builder to be used in the PR message if show_in_pr is true
notices: @notices
)

# Send warning alerts to the API if any warning notices are present.
# Note that only notices with notice.show_alert set to true will be sent.
record_warning_notices(notices) if notices.any?

# NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive
# and the dependency name in the security advisory often doesn't match
# what users have specified in their manifest.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class Updater
module Operations
class RefreshVersionUpdatePullRequest
extend T::Sig
include PullRequestHelpers

sig { params(job: Dependabot::Job).returns(T::Boolean) }
def self.applies_to?(job:)
Expand Down Expand Up @@ -146,9 +147,14 @@ def check_and_update_pull_request(dependencies)
dependency_files: dependency_snapshot.dependency_files,
updated_dependencies: updated_deps,
change_source: checker.dependency,
# Sending notices to the pr message builder to be used in the PR message if show_in_pr is true
notices: @notices
)

# Send warning alerts to the API if any warning notices are present.
# Note that only notices with notice.show_alert set to true will be sent.
record_warning_notices(notices) if notices.any?

# NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive
# and the dependency name in the security advisory often doesn't match
# what users have specified in their manifest.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Updater
module Operations
class UpdateAllVersions
extend T::Sig
include PullRequestHelpers

sig { params(_job: Dependabot::Job).returns(T::Boolean) }
def self.applies_to?(_job:)
Expand Down Expand Up @@ -167,13 +168,18 @@ def check_and_create_pull_request(dependency)
dependency_files: dependency_snapshot.dependency_files,
updated_dependencies: updated_deps,
change_source: checker.dependency,
# Sending notices to the pr message builder to be used in the PR message if show_in_pr is true
notices: @notices
)

if dependency_change.updated_dependency_files.empty?
raise "UpdateChecker found viable dependencies to be updated, but FileUpdater failed to update any files"
end

# Send warning alerts to the API if any warning notices are present.
# Note that only notices with notice.show_alert set to true will be sent.
record_warning_notices(notices) if notices.any?

create_pull_request(dependency_change)
end
# rubocop:enable Metrics/PerceivedComplexity
Expand Down
47 changes: 47 additions & 0 deletions updater/lib/dependabot/updater/security_update_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,5 +181,52 @@ def security_update_not_possible_message(checker, latest_allowed_version, confli
end
end
end

module PullRequestHelpers
extend T::Sig
extend T::Helpers

sig { returns(Dependabot::Service) }
attr_reader :service

abstract!

sig { params(notices: T.nilable(T::Array[Dependabot::Notice])).void }
def record_warning_notices(notices)
return if !notices || notices.empty?

# Find unique warning notices which are going to be shown on insight page.
warn_notices = unique_warn_notices(notices)

warn_notices.each do |notice|
# If alert is enabled, sending the deprecation notice to the service for showing on the UI insight page
send_alert_notice(notice) if notice.show_alert
end
rescue StandardError => e
Dependabot.logger.error(
"Failed to send notice warning: #{e.message}"
)
end

private

# Resurns unique warning notices which are going to be shown on insight page.
sig { params(notices: T::Array[Dependabot::Notice]).returns(T::Array[Dependabot::Notice]) }
def unique_warn_notices(notices)
notices
.select { |notice| notice.mode == Dependabot::Notice::NoticeMode::WARN }
.uniq { |notice| [notice.type, notice.package_manager_name] }
end

sig { params(notice: Dependabot::Notice).void }
def send_alert_notice(notice)
# Sending the notice to the service for showing on the dependabot insight page
service.record_update_job_warning(
warn_type: notice.type,
warn_title: notice.title,
warn_description: notice.description
)
end
end
end
end
41 changes: 41 additions & 0 deletions updater/spec/dependabot/api_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,47 @@
end
end

describe "record_update_job_warning" do
let(:record_update_job_warning_url) { "http://example.com/update_jobs/1/record_update_job_warning" }

let(:warn_type) { "test_warning_type" }
let(:warn_title) { "Test Warning Title" }
let(:warn_description) { "Test Warning Description" }

before do
stub_request(:post, record_update_job_warning_url)
.to_return(status: 204, headers: headers)
end

it "hits the correct endpoint" do
client.record_update_job_warning(
warn_type: warn_type,
warn_title: warn_title,
warn_description: warn_description
)

expect(WebMock)
.to have_requested(:post, record_update_job_warning_url)
.with(headers: { "Authorization" => "token" })
end

it "encodes the payload correctly" do
client.record_update_job_warning(
warn_type: warn_type,
warn_title: warn_title,
warn_description: warn_description
)

expect(WebMock).to(have_requested(:post, record_update_job_warning_url).with do |req|
data = JSON.parse(req.body)["data"]

expect(data["warn-type"]).to eq(warn_type)
expect(data["warn-title"]).to eq(warn_title)
expect(data["warn-description"]).to eq(warn_description)
end)
end
end

describe "mark_job_as_processed" do
let(:url) { "http://example.com/update_jobs/1/mark_as_processed" }
let(:base_commit) { "sha" }
Expand Down
25 changes: 24 additions & 1 deletion updater/spec/dependabot/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
update_pull_request: nil,
close_pull_request: nil,
record_update_job_error: nil,
record_update_job_unknown_error: nil
record_update_job_unknown_error: nil,
record_update_job_warning: nil
})
allow(api_client).to receive(:is_a?).with(Dependabot::ApiClient).and_return(true)
api_client
Expand Down Expand Up @@ -305,6 +306,28 @@
end
end

describe "#record_update_job_warning" do
let(:warn_type) { :deprecated_dependency }
let(:warn_title) { "Deprecated Dependency Used" }
let(:warn_description) { "The dependency xyz is deprecated and should be updated or removed." }

before do
service.record_update_job_warning(
warn_type: warn_type,
warn_title: warn_title,
warn_description: warn_description
)
end

it "delegates to @client" do
expect(mock_client).to have_received(:record_update_job_warning).with(
warn_type: warn_type,
warn_title: warn_title,
warn_description: warn_description
)
end
end

describe "#capture_exception" do
before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Expand Down
Loading

0 comments on commit 921363f

Please sign in to comment.