[SINT-5051] Update CI identities client version and Windows code signer to latest release#8474
Conversation
andrewlock
left a comment
There was a problem hiding this comment.
Thanks, more changes are required to actually make these changes live though (see the comment at the top of the dockerfile)
I'll look at rebuilding and pushing this image soon, or better, maybe I'll try to update things so we can build directly in gitlab instead...
BenchmarksBenchmark execution time: 2026-05-06 13:45:44 Comparing candidate commit 05da2b0 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 58 known flaky benchmarks, 29 flaky benchmarks without significant changes.
|
## Summary of changes - Moves the building of GitLab image into GitLab - Updates the build images/runners to use Windows 2022 ## Reason for change Today, updating the windows gitlab image involves - building the image locally - pushing it to dockerhub - testing the image works - making a PR to Datadog/images to mirror the image - Updating the original PR to use the new image That's a lot of faff. Instead, this PR changes it so that building the docker image is just "click a button in GitLab" ## Implementation details The crux of the implementation uses a hash of the contents of the gitlab dockerimage folder. There's other ways we could do this, but this has the nice behaviour of meaning your build will fail if you change something there and _don't_ rebuild the image (e.g. see #8474). The first time you change something in this folder, [the `Build` stage will fail](https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-dotnet/-/jobs/1621141095): ``` ERROR: Windows build image not found at registry.ddbuild.io/ci/dd-trace-dotnet/dd-trace-dotnet-docker-build:d8e89e5cffc8. The Dockerfile or install scripts under tracer/build/_build/docker/gitlab/ have changed. Manually trigger the 'build-windows-ci-image' job in this pipeline, wait for it to finish, then re-run this job. ``` You can then click on the pipeline, click [build-windows-ci-image](https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-dotnet/-/jobs/1621141094), and go make a coffee ☕ After ~45 mins, the new image should have been pushed, and you can re-run the build, with your new changes. ## Test coverage I tested the workflow for this PR, and it works 🙌 ## Other details One issue I had to deal with is the 2019/2022 issue. You can only run docker images of the same base Windows version on the host, and the `registry.ddbuild.io/ci-identities/ci-identities-gitlab-job-client:v0.2.0-windows-amd64` image is apparently a 2022 host. Seeing as we really should be using 2022 anyway (and we use 2022 in azure devops), switched everything to be 2022 instead
da5772a to
05da2b0
Compare
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8474) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (73ms) : 69, 77
master - mean (73ms) : 71, 76
section Bailout
This PR (8474) - mean (80ms) : 77, 84
master - mean (77ms) : 75, 80
section CallTarget+Inlining+NGEN
This PR (8474) - mean (1,081ms) : 1042, 1121
master - mean (1,086ms) : 1035, 1138
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (117ms) : 111, 123
master - mean (114ms) : 109, 119
section Bailout
This PR (8474) - mean (117ms) : 111, 123
master - mean (118ms) : 111, 126
section CallTarget+Inlining+NGEN
This PR (8474) - mean (781ms) : 756, 806
master - mean (786ms) : 762, 811
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (104ms) : 98, 110
master - mean (100ms) : 97, 104
section Bailout
This PR (8474) - mean (105ms) : 99, 110
master - mean (103ms) : 98, 107
section CallTarget+Inlining+NGEN
This PR (8474) - mean (941ms) : 910, 973
master - mean (940ms) : 903, 977
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (100ms) : 97, 102
master - mean (103ms) : 96, 109
section Bailout
This PR (8474) - mean (105ms) : 98, 111
master - mean (101ms) : 99, 104
section CallTarget+Inlining+NGEN
This PR (8474) - mean (822ms) : 783, 862
master - mean (823ms) : 776, 870
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (209ms) : 197, 222
master - mean (206ms) : 193, 219
section Bailout
This PR (8474) - mean (213ms) : 202, 225
master - mean (210ms) : 200, 220
section CallTarget+Inlining+NGEN
This PR (8474) - mean (1,239ms) : 1178, 1300
master - mean (1,224ms) : 1168, 1281
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (297ms) : 278, 316
master - mean (296ms) : 280, 311
section Bailout
This PR (8474) - mean (298ms) : 279, 317
master - mean (298ms) : 280, 316
section CallTarget+Inlining+NGEN
This PR (8474) - mean (997ms) : 973, 1021
master - mean (981ms) : 943, 1020
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (293ms) : 268, 317
master - mean (299ms) : 279, 319
section Bailout
This PR (8474) - mean (292ms) : 273, 311
master - mean (296ms) : 279, 312
section CallTarget+Inlining+NGEN
This PR (8474) - mean (1,176ms) : 1126, 1226
master - mean (1,175ms) : 1129, 1222
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8474) - mean (294ms) : 273, 315
master - mean (295ms) : 278, 313
section Bailout
This PR (8474) - mean (295ms) : 277, 312
master - mean (295ms) : 280, 311
section CallTarget+Inlining+NGEN
This PR (8474) - mean (1,068ms) : 990, 1146
master - mean (1,074ms) : 976, 1171
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
andrewlock
left a comment
There was a problem hiding this comment.
Thanks! Sorry for the delay, I worked on #8507 to make actually shipping this change easier, and rebased. The resulting rebuild seems to have worked so 🤞 all looks good at your end!
Summary of changes
Reason for change
Mostly for better observability
See changelog : link