dd-trace-php icon indicating copy to clipboard operation
dd-trace-php copied to clipboard

test: add missing CI image to mirror

Open Leiyks opened this issue 1 month ago • 4 comments

Description

This PR modify our CI to use our private registry instead of dockerhub to pull the public images we are using. The goal is to stop having image pull rate limits errors when we are running too many pipeline.

Leiyks avatar Nov 28 '25 16:11 Leiyks

Benchmarks [ tracer ]

Benchmark execution time: 2025-12-09 12:08:19

Comparing candidate commit e20dc42ebb241310249b1cff22ac85d33b49f329 in PR branch leiyks/fix-ci-docker-pull-limit with baseline commit 9468244b4bb4f3a1eea5a29c631e935fbd9eba13 in branch master.

Found 2 performance improvements and 1 performance regressions! Performance is the same for 190 metrics, 1 unstable metrics.

scenario:EmptyFileBench/benchEmptyFileDdprof-opcache

  • 🟩 execution_time [-523.940µs; -244.580µs] or [-13.033%; -6.084%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟥 execution_time [+3.128µs; +4.292µs] or [+2.956%; +4.058%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟩 execution_time [-36.086µs; -20.114µs] or [-8.156%; -4.546%]

pr-commenter[bot] avatar Dec 09 '25 12:12 pr-commenter[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 61.64%. Comparing base (373e64d) to head (54da5c7).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3510      +/-   ##
==========================================
- Coverage   61.75%   61.64%   -0.11%     
==========================================
  Files         143      143              
  Lines       13008    13008              
  Branches     1702     1702              
==========================================
- Hits         8033     8019      -14     
- Misses       4215     4228      +13     
- Partials      760      761       +1     

see 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 373e64d...54da5c7. Read the comment docs.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov-commenter avatar Dec 09 '25 13:12 codecov-commenter

Have we considered authenticating against Docker Hub to bypass (or widen) pull limits? From a technical perspective this is fine, from a maintenance perspective I have questions:

  • will images in our mirror be updated magically?
  • if not: could we describe the process in a doc in this repo somewhere?

I am asking because this will add maintenance burden to us and we should be clear about that, if it's the case

realFlowControl avatar Dec 09 '25 13:12 realFlowControl

Have we considered authenticating against Docker Hub to bypass (or widen) pull limits?

Indeed, this could be a much better solution under the condition that we have a Pro/Team/Business service account that we can use to do the authentication. Currently to upload docker images we are launching a manual pipeline with our account name and access token from dockerhub. After discussing with Pawel we figured that just using our private registry would be fine, but I agree that it would cause an issue from a maintenance perspective.

In case we continue with that solution, we can just update those image when it cause an issue with the CI. This is currently what we are doing with some public images that are already mirrored into our private registry, and since we need to add some proper alerting to our CI later it won't go unnoticed

Leiyks avatar Dec 09 '25 14:12 Leiyks

Should we close in favour of https://github.com/DataDog/dd-trace-php/pull/3536 ?

realFlowControl avatar Dec 18 '25 13:12 realFlowControl

Should we close in favour of #3536 ?

Yes, let's close this one :+1:

Leiyks avatar Dec 18 '25 13:12 Leiyks