From b7f14e3f1574c5707485e4d6a28c8b30ec3688df Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 28 Feb 2024 22:00:42 +0000 Subject: [PATCH] Address PR comments --- updater/lib/dependabot/api_client.rb | 19 +++++++++---------- .../lib/dependabot/file_fetcher_command.rb | 2 +- updater/lib/dependabot/opentelemetry.rb | 6 ++++-- .../lib/dependabot/update_files_command.rb | 2 +- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/updater/lib/dependabot/api_client.rb b/updater/lib/dependabot/api_client.rb index 1f0b5cde4e7..4efdc73dca2 100644 --- a/updater/lib/dependabot/api_client.rb +++ b/updater/lib/dependabot/api_client.rb @@ -31,7 +31,7 @@ def initialize(base_url, job_id, job_token) # TODO: Make `base_commit_sha` part of Dependabot::DependencyChange sig { params(dependency_change: Dependabot::DependencyChange, base_commit_sha: String).void } def create_pull_request(dependency_change, base_commit_sha) - ::Dependabot::OpenTelemetry.tracer&.in_span("create_pull_request", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("create_pull_request", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID, job_id.to_s) span.set_attribute(::Dependabot::OpenTelemetry::Attributes::BASE_COMMIT_SHA, base_commit_sha) span.set_attribute(::Dependabot::OpenTelemetry::Attributes::DEPENDENCY_NAMES, dependency_change.humanized) @@ -54,7 +54,7 @@ def create_pull_request(dependency_change, base_commit_sha) # TODO: Determine if we should regenerate the PR message within core for updates sig { params(dependency_change: Dependabot::DependencyChange, base_commit_sha: String).void } def update_pull_request(dependency_change, base_commit_sha) - ::Dependabot::OpenTelemetry.tracer&.in_span("update_pull_request", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("update_pull_request", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID, job_id.to_s) span.set_attribute(::Dependabot::OpenTelemetry::Attributes::BASE_COMMIT_SHA, base_commit_sha) span.set_attribute(::Dependabot::OpenTelemetry::Attributes::DEPENDENCY_NAMES, dependency_change.humanized) @@ -81,7 +81,7 @@ def update_pull_request(dependency_change, base_commit_sha) sig { params(dependency_names: T.any(String, T::Array[String]), reason: T.any(String, Symbol)).void } def close_pull_request(dependency_names, reason) - ::Dependabot::OpenTelemetry.tracer&.in_span("close_pull_request", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("close_pull_request", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID, job_id.to_s) span.set_attribute(::Dependabot::OpenTelemetry::Attributes::PR_CLOSE_REASON, reason.to_s) @@ -101,10 +101,9 @@ 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.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) - api_url = "#{base_url}/update_jobs/#{job_id}/record_update_job_error" body = { data: { @@ -127,7 +126,7 @@ def record_update_job_error(error_type:, error_details:) 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? - ::Dependabot::OpenTelemetry.tracer&.in_span("record_update_job_unknown_error", kind: :internal) do |_span| + ::Dependabot::OpenTelemetry.tracer.in_span("record_update_job_unknown_error", kind: :internal) do |_span| ::Dependabot::OpenTelemetry.record_update_job_error(job_id: job_id, error_type: error_type, error_details: error_details) @@ -152,7 +151,7 @@ def record_update_job_unknown_error(error_type:, error_details:) sig { params(base_commit_sha: String).void } def mark_job_as_processed(base_commit_sha) - ::Dependabot::OpenTelemetry.tracer&.in_span("mark_job_as_processed", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("mark_job_as_processed", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::BASE_COMMIT_SHA, base_commit_sha) span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID, job_id.to_s) @@ -172,7 +171,7 @@ def mark_job_as_processed(base_commit_sha) sig { params(dependencies: T::Array[T::Hash[Symbol, T.untyped]], dependency_files: T::Array[String]).void } def update_dependency_list(dependencies, dependency_files) - ::Dependabot::OpenTelemetry.tracer&.in_span("update_dependency_list", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("update_dependency_list", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID, job_id.to_s) api_url = "#{base_url}/update_jobs/#{job_id}/update_dependency_list" @@ -196,7 +195,7 @@ def update_dependency_list(dependencies, dependency_files) sig { params(ecosystem_versions: T::Hash[Symbol, T.untyped]).void } def record_ecosystem_versions(ecosystem_versions) - ::Dependabot::OpenTelemetry.tracer&.in_span("record_ecosystem_versions", kind: :internal) do |_span| + ::Dependabot::OpenTelemetry.tracer.in_span("record_ecosystem_versions", kind: :internal) do |_span| api_url = "#{base_url}/update_jobs/#{job_id}/record_ecosystem_versions" body = { data: { ecosystem_versions: ecosystem_versions } @@ -215,7 +214,7 @@ def record_ecosystem_versions(ecosystem_versions) sig { params(metric: String, tags: T::Hash[String, String]).void } def increment_metric(metric, tags:) - ::Dependabot::OpenTelemetry.tracer&.in_span("increment_metric", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("increment_metric", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID.to_s, job_id.to_s) span.set_attribute(::Dependabot::OpenTelemetry::Attributes::METRIC.to_s, metric) tags.each do |key, value| diff --git a/updater/lib/dependabot/file_fetcher_command.rb b/updater/lib/dependabot/file_fetcher_command.rb index ae6e402cbb9..a3c669ce336 100644 --- a/updater/lib/dependabot/file_fetcher_command.rb +++ b/updater/lib/dependabot/file_fetcher_command.rb @@ -18,7 +18,7 @@ def perform_job # rubocop:disable Metrics/PerceivedComplexity,Metrics/AbcSize @base_commit_sha = nil Dependabot.logger.info("Job definition: #{File.read(Environment.job_path)}") if Environment.job_path - ::Dependabot::OpenTelemetry.tracer&.in_span("file_fetcher", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("file_fetcher", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID, job_id.to_s) begin diff --git a/updater/lib/dependabot/opentelemetry.rb b/updater/lib/dependabot/opentelemetry.rb index 3ee1a475fb5..3d89b654628 100644 --- a/updater/lib/dependabot/opentelemetry.rb +++ b/updater/lib/dependabot/opentelemetry.rb @@ -25,7 +25,9 @@ def self.should_configure? sig { void } def self.configure - puts "Configuring OpenTelemetry..." + return unless should_configure? + + puts "OpenTelemetry is enabled, configuring..." require "opentelemetry/exporter/otlp" @@ -52,7 +54,7 @@ def self.configure tracer end - sig { returns(T.nilable(::OpenTelemetry::Trace::Tracer)) } + sig { returns(::OpenTelemetry::Trace::Tracer) } def self.tracer ::OpenTelemetry.tracer_provider.tracer("dependabot", Dependabot::VERSION) end diff --git a/updater/lib/dependabot/update_files_command.rb b/updater/lib/dependabot/update_files_command.rb index 928382f8ceb..7a1b5729568 100644 --- a/updater/lib/dependabot/update_files_command.rb +++ b/updater/lib/dependabot/update_files_command.rb @@ -14,7 +14,7 @@ def perform_job # encoded files and commit information in the environment, so let's retrieve # them, decode and parse them into an object that knows the current state # of the project's dependencies. - ::Dependabot::OpenTelemetry.tracer&.in_span("update_files", kind: :internal) do |span| + ::Dependabot::OpenTelemetry.tracer.in_span("update_files", kind: :internal) do |span| span.set_attribute(::Dependabot::OpenTelemetry::Attributes::JOB_ID, job_id.to_s) begin