Skip to content

Commit c1516a5

Browse files
Copilotpavera
andauthored
Remove enable_record_ecosystem_meta feature flag (#14353)
* Remove enable_record_ecosystem_meta feature flag The record_ecosystem_meta functionality is now always active (no longer gated behind an experiment flag). Removed: - Feature flag guard in api_client.rb - Associated test stubs and "when feature flag is disabled" test context Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Fix lint and integration test failures - Fix RuboCop Layout/EmptyLinesAroundBlockBody offense (extra empty line) - Add stub_request for record_ecosystem_meta_url in spec - Fix Go integration test: check message type before full struct unmarshal so record_ecosystem_meta messages (which have different data structure) are skipped gracefully instead of causing unmarshal errors Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Revert accidentally committed go_modules fixture files Remove go.sum/go.mod fixture files that were accidentally picked up by git add and are not related to the feature flag removal. Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Make record_ecosystem_meta best-effort without retries Remove the retry loop from record_ecosystem_meta to avoid adding up to ~30s of extra latency per job when the API call fails. Since this is now unconditional and invoked from multiple ensure blocks, it should be truly best-effort (like increment_metric) with no sleep/retry behavior. Network errors and HTTP 4xx+ responses are logged as errors without retrying. Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Simplify record_ecosystem_meta error handling Remove redundant inner rescue for Excon/SSL errors since the outer StandardError rescue already catches those. Single rescue block is cleaner and matches the reviewer's suggested pattern. Co-authored-by: pavera <660677+pavera@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pavera <660677+pavera@users.noreply.github.com>
1 parent 3ca8498 commit c1516a5

3 files changed

Lines changed: 33 additions & 52 deletions

File tree

silent/tests/silent_test.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,20 @@ func prChecker(prType string) func(s *script.State, args ...string) (script.Wait
102102
scanner := bufio.NewScanner(strings.NewReader(s.Stdout()))
103103
var totalPRsCreated int
104104
for scanner.Scan() {
105+
var msg struct {
106+
Type string `json:"type"`
107+
}
108+
if err := json.Unmarshal([]byte(scanner.Text()), &msg); err != nil {
109+
return nil, fmt.Errorf("failed to decode line %s: %w", scanner.Text(), err)
110+
}
111+
if msg.Type != prType {
112+
continue
113+
}
105114
var pr CreatePR
106115
err := json.Unmarshal([]byte(scanner.Text()), &pr)
107116
if err != nil {
108117
return nil, fmt.Errorf("failed to decode line %s: %w", scanner.Text(), err)
109118
}
110-
if pr.Type != prType {
111-
continue
112-
}
113119

114120
totalPRsCreated++
115121
if len(pr.Data.UpdatedDependencyFiles) == 0 {

updater/lib/dependabot/api_client.rb

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -308,46 +308,32 @@ def increment_metric(metric, tags:)
308308

309309
sig { params(ecosystem: T.nilable(Ecosystem)).void }
310310
def record_ecosystem_meta(ecosystem)
311-
return unless Dependabot::Experiments.enabled?(:enable_record_ecosystem_meta)
312-
313311
return if ecosystem.nil?
314312

315-
begin
316-
::Dependabot::OpenTelemetry.tracer.in_span("record_ecosystem_meta", kind: :internal) do |_span|
317-
api_url = "#{http_client.params[:path]}/update_jobs/#{job_id}/record_ecosystem_meta"
313+
::Dependabot::OpenTelemetry.tracer.in_span("record_ecosystem_meta", kind: :internal) do |_span|
314+
api_url = "#{http_client.params[:path]}/update_jobs/#{job_id}/record_ecosystem_meta"
318315

319-
body = {
320-
data: [
321-
{
322-
ecosystem: {
323-
name: ecosystem.name,
324-
package_manager: version_manager_json(ecosystem.package_manager),
325-
language: version_manager_json(ecosystem.language)
326-
}
316+
body = {
317+
data: [
318+
{
319+
ecosystem: {
320+
name: ecosystem.name,
321+
package_manager: version_manager_json(ecosystem.package_manager),
322+
language: version_manager_json(ecosystem.language)
327323
}
328-
]
329-
}
330-
331-
retry_count = 0
324+
}
325+
]
326+
}
332327

333-
begin
334-
response = http_client.post(path: api_url, body: body.to_json)
335-
raise ApiError, response.body if response.status >= 400
336-
rescue Excon::Error::Socket, Excon::Error::Timeout, OpenSSL::SSL::SSLError, ApiError => e
337-
retry_count += 1
338-
if retry_count <= MAX_REQUEST_RETRIES
339-
sleep(rand(3.0..10.0))
340-
retry
341-
else
342-
Dependabot.logger.error(
343-
"Failed to record ecosystem meta after #{MAX_REQUEST_RETRIES} retries: #{e.message}"
344-
)
345-
end
346-
end
328+
response = http_client.post(path: api_url, body: body.to_json)
329+
if response.status >= 400
330+
Dependabot.logger.error(
331+
"Failed to record ecosystem meta. Status: #{response.status}, body: #{response.body}"
332+
)
347333
end
348-
rescue StandardError => e
349-
Dependabot.logger.error("Failed to record ecosystem meta: #{e.message}")
350334
end
335+
rescue StandardError => e
336+
Dependabot.logger.error("Failed to record ecosystem meta: #{e.message}")
351337
end
352338

353339
# rubocop:disable Metrics/MethodLength

updater/spec/dependabot/api_client_spec.rb

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@
8383

8484
before do
8585
allow(Dependabot::PullRequestCreator::MessageBuilder).to receive_message_chain(:new, :message).and_return(message)
86-
allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_record_ecosystem_meta).and_return(true)
8786
stub_request(:post, create_pull_request_url)
8887
.to_return(status: 204, headers: headers)
8988
end
@@ -651,10 +650,6 @@
651650
end
652651

653652
describe "record_ecosystem_meta" do
654-
before do
655-
allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_record_ecosystem_meta).and_return(true)
656-
end
657-
658653
let(:ecosystem) do
659654
Dependabot::Ecosystem.new(
660655
name: "bundler",
@@ -683,6 +678,11 @@
683678
end
684679
let(:record_ecosystem_meta_url) { "http://example.com/update_jobs/1/record_ecosystem_meta" }
685680

681+
before do
682+
stub_request(:post, record_ecosystem_meta_url)
683+
.to_return(status: 204, headers: headers)
684+
end
685+
686686
it "hits the correct endpoint" do
687687
client.record_ecosystem_meta(ecosystem)
688688

@@ -726,17 +726,6 @@
726726
expect(WebMock).not_to have_requested(:post, record_ecosystem_meta_url)
727727
end
728728
end
729-
730-
context "when feature flag is disabled" do
731-
before do
732-
allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_record_ecosystem_meta).and_return(false)
733-
end
734-
735-
it "does not send a request" do
736-
client.record_ecosystem_meta(ecosystem)
737-
expect(WebMock).not_to have_requested(:post, record_ecosystem_meta_url)
738-
end
739-
end
740729
end
741730

742731
describe "record_cooldown_meta" do

0 commit comments

Comments
 (0)