Send Ecosystem Metrics to Dependabot-API on Update Job Completion #10905

kbukum1 · 2024-11-07T01:02:45Z

What are you trying to accomplish?

This PR adds functionality, controlled by the enable_record_ecosystem_meta feature flag, to send ecosystem metrics from update_job results to Dependabot-API. This allows for improved tracking of dependency updates across ecosystems, including versioning details for package managers and languages. Metrics cover raw and processed versions as well as constraints, enabling better visibility into compatibility trends.

What issues does this affect or fix?

This change is part of ongoing efforts to enhance update visibility and track deprecations across ecosystems.

Anything you want to highlight for special attention from reviewers?

The ecosystem_data method structures and extracts version and requirement details. Data includes nested fields, which may need flattening for Datadog compatibility. Reviewers should verify that the metric structure aligns with Dependabot-API requirements.

How will you know you've accomplished your goal?

Ecosystem metrics appear in Dependabot-API logs as expected when the feature flag is enabled.
Data structure supports compatibility with Datadog for effective tracking.
Test coverage includes validation across different ecosystems and constraints.

Checklist

I have run the test suite to ensure all tests and linters pass.
I have verified code changes and added tests for new functionality.
I have written clear, descriptive commit messages.
I have provided a detailed description of the changes, including problem context, implementation, and impact.
Code is well-documented and easy to understand.

kbukum1 · 2024-11-08T00:13:50Z

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


Review Tip: This PR addresses type issues introduced in the previous PR by ensuring all versions consistently use Dependabot::Version instead of the more generic Gem::Version. This change provides more specific typing, improving clarity and reducing abstraction.

Previous PR: Expand Centralized Ecosystem Format with Requirements Information for Bundler Package Manager #10897

kbukum1 · 2024-11-08T01:20:14Z

updater/lib/dependabot/api_client.rb

@@ -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



Review Tip: Moved retry request count in constant variable since it is always equal 3.

kbukum1 · 2024-11-08T01:21:15Z

updater/lib/dependabot/api_client.rb

+        Dependabot.logger.error("Failed to record ecosystem meta: #{e.message}")
+      end
+    end
+


Review Tip: Create metrics from ecosystem instance that stores ecosystem, package manager and language information.

kbukum1 · 2024-11-08T01:21:52Z

updater/lib/dependabot/api_client.rb

+        max_version: requirement.max_version&.to_s
+      }
+    end
+


Review Tip: Use common metrics creation functions for package manager and language since they have metrics in same structure.

kbukum1 · 2024-11-08T01:22:44Z

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


Review Tip: Send metrics for group refresh operation after update process (success, or fail)

updater/lib/dependabot/updater/group_update_refreshing.rb

kbukum1 · 2024-11-08T01:23:25Z

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


Review Tip: Send metrics for create group operation after update process

kbukum1 · 2024-11-08T01:24:37Z

updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb

+            # - an update is available (has_update = true), or
+            # - an error occurred (has_update = nil)
+            service.record_ecosystem_meta(dependency_snapshot.ecosystem) if has_update != false
+          end
        end


Review Tip: Send metrics for create security update after update process (success, or fail). Just being sure that metrics is getting called when there is update. We don't prefer to send metrics when there is not update necessary

@kbukum1: Not logging no update necessary events simplifies logging but I would suggest, we should log no update as well. The reason being without logging no update for Dependabot job run, makes it hard to gauge the baseline activity level (example: lose the ability to compare how often checks lead to updates versus when no updates are necessary). Once we have this data we can reason with it. Same will be true at different places where you are sending metrics.

@abdulapopoola what do you think. Should we log all update processes? This may be easier to implement but for each update process we are going to send metrics even through there may be no update

@honeyankit ,

Just wanted to mention, normally we discussed that when we were preparing the ADR and we thought it will be ok to start with only if there is update. We were thinking to add no-update later if we see is necessary.

@honeyankit ,

It is fixed now. I am sending metrics if update process started for a dependency or a group even there is no update.

kbukum1 · 2024-11-08T01:25:49Z

updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb

@@ -190,6 +219,7 @@ def check_and_create_pull_request(dependency)
          record_warning_notices(notices) if notices.any?

          create_pull_request(dependency_change)
+          true
        rescue Dependabot::AllVersionsIgnored


Review Tip: Using boolean to understand if update process (succeed, fail, no update)

kbukum1 · 2024-11-08T01:26:11Z

updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb

+            # - an update is available (has_update = true), or
+            # - an error occurred (has_update = nil)
+            service.record_ecosystem_meta(dependency_snapshot.ecosystem) if has_update != false
+          end


Review Tip: Send metrics for refresh version update after update process (success, or fail). Just being sure that metrics is getting called when there is update. We don't prefer to send metrics when there is not update necessary

kbukum1 · 2024-11-08T01:26:26Z

updater/lib/dependabot/updater/operations/update_all_versions.rb

+            # - an update is available (has_update = true), or
+            # - an error occurred (has_update = nil)
+            service.record_ecosystem_meta(dependency_snapshot.ecosystem) if has_update != false
+          end
        end


Review Tip: Send metrics for update all after update process. Just being sure that metrics is getting called when there is update. We don't prefer to send metrics when there is not update necessary

amazimbe · 2024-11-11T11:56:04Z

updater/lib/dependabot/api_client.rb

+          api_url = "#{base_url}/update_jobs/#{job_id}/record_ecosystem_meta"
+
+          body = {
+            data: {


Just to note that this was an array in the ADR and that's the format the new endpoint expects.

Thanks. I will change it as array then.

It is fixed.

kbukum1 changed the title ~~Send Update Job Ecosystem Metrics to Dependabot-API~~ Send Ecosystem Metrics to Dependabot-API on Update Job Completion Nov 7, 2024

kbukum1 commented Nov 8, 2024

View reviewed changes

kbukum1 force-pushed the kamil/send_update_job_ecosyste_metrics_to_dependabot_api branch from d7936ef to fcf6572 Compare November 8, 2024 00:53

kbukum1 commented Nov 8, 2024

View reviewed changes

updater/lib/dependabot/updater/group_update_refreshing.rb Show resolved Hide resolved

kbukum1 commented Nov 8, 2024

View reviewed changes

kbukum1 marked this pull request as ready for review November 8, 2024 19:13

kbukum1 requested a review from a team as a code owner November 8, 2024 19:13

kbukum1 force-pushed the kamil/send_update_job_ecosyste_metrics_to_dependabot_api branch from 5cb9de1 to 3d25baf Compare November 8, 2024 22:07

kbukum1 requested review from abdulapopoola and honeyankit November 8, 2024 22:10

amazimbe reviewed Nov 11, 2024

View reviewed changes

kbukum1 force-pushed the kamil/send_update_job_ecosyste_metrics_to_dependabot_api branch from 3b01cca to 5c76a35 Compare November 12, 2024 19:38

send ecosystem metrics to dependabot-api

67643bf

kbukum1 force-pushed the kamil/send_update_job_ecosyste_metrics_to_dependabot_api branch from 5c76a35 to 67643bf Compare November 12, 2024 19:59

kbukum1 requested a review from amazimbe November 12, 2024 20:14

honeyankit approved these changes Nov 12, 2024

View reviewed changes

kbukum1 added 2 commits November 12, 2024 15:58

add sorbet nil check

5d72088

added spec when ecosystem is nil for record_ecosystem_meta

3ee82ca

abdulapopoola approved these changes Nov 13, 2024

View reviewed changes

abdulapopoola removed the request for review from amazimbe November 13, 2024 19:28

kbukum1 merged commit 39df7e8 into main Nov 13, 2024
147 checks passed

kbukum1 deleted the kamil/send_update_job_ecosyste_metrics_to_dependabot_api branch November 13, 2024 19:39

kbukum1 self-assigned this Nov 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send Ecosystem Metrics to Dependabot-API on Update Job Completion #10905

Send Ecosystem Metrics to Dependabot-API on Update Job Completion #10905

kbukum1 commented Nov 7, 2024 •

edited

Loading

kbukum1 Nov 8, 2024

kbukum1 Nov 8, 2024

kbukum1 Nov 8, 2024

kbukum1 Nov 8, 2024

kbukum1 Nov 8, 2024

kbukum1 Nov 8, 2024 •

edited

Loading

kbukum1 Nov 8, 2024

honeyankit Nov 8, 2024

kbukum1 Nov 12, 2024

kbukum1 Nov 12, 2024 •

edited

Loading

kbukum1 Nov 12, 2024 •

edited

Loading

kbukum1 Nov 8, 2024

kbukum1 Nov 8, 2024

kbukum1 Nov 8, 2024 •

edited

Loading

amazimbe Nov 11, 2024

kbukum1 Nov 12, 2024

kbukum1 Nov 12, 2024

Send Ecosystem Metrics to Dependabot-API on Update Job Completion #10905

Send Ecosystem Metrics to Dependabot-API on Update Job Completion #10905

Conversation

kbukum1 commented Nov 7, 2024 • edited Loading

What are you trying to accomplish?

What issues does this affect or fix?

Anything you want to highlight for special attention from reviewers?

How will you know you've accomplished your goal?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

kbukum1 Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 commented Nov 7, 2024 •

edited

Loading

kbukum1 Nov 8, 2024 •

edited

Loading

kbukum1 Nov 12, 2024 •

edited

Loading

kbukum1 Nov 12, 2024 •

edited

Loading

kbukum1 Nov 8, 2024 •

edited

Loading