ci: bump apm-sdks-benchmarks pin to include S3 upload support - #12531
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
Conversation
The ref pinned for the five apm-sdks-benchmarks includes (spring-petclinic,
load, startup, dacapo, post-pr-comment) predates apm-sdks-benchmarks
commit 346ee41 ("feat: upload parallel benchmark results to S3 (#251)"),
so .gitlab/templates/upload-results.yml does not exist at that commit and
these jobs' S3 upload path is broken on master today.
Already validated on a separate branch, augusto/test-java-macrobenchmark-stability,
pinned to an earlier post-346ee41 commit: a real stability-monitoring run
there collected 500 .converted.json files across 10 samples, 103 scenarios,
14 metrics. This bumps master itself to current apm-sdks-benchmarks main.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
More details
The new SHA remains on the shared anchor, so all five benchmark includes use the same revision.
🤖 Datadog Autotest · Commit 511e454 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
amarziali
left a comment
There was a problem hiding this comment.
nit: Please keep the pin comment durable across automated updates. The weekly update script replaces only the SHA, so the current “apm-sdks-benchmarks#261 merge commit” text will become stale on the next bump. Restoring the generic “pinned by Update apm-sdks-benchmarks reference workflow” wording would avoid that. This is non-blocking.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
|
/merge |
|
View all feedbacks in Devflow UI.
PR already in the queue with status in_progress |
40d1adc
into
master
What Does This Do
Bumps the
apm-sdks-benchmarksinclude pin (&apm_sdks_benchmarks_sha, shared by all five includes: spring-petclinic, load, startup, dacapo, post-pr-comment) from0899730bto currentapm-sdks-benchmarksmainHEAD (fac3cc72).Motivation
The old pin predates
346ee41(uploading results to S3), and we need it for monitoring benchmark flakiness: https://github.com/DataDog/benchmarking-platform-tools/tree/main/scripts/stability#stability-monitoring-ciAdditional Notes
Two open PRs from the "Update apm-sdks-benchmarks reference" automation (#12418, #12479) attempt the same bump. Merging either of those or this PR resolves it.
Jira ticket: APMSP-3925
How to test the change?
Already validated that the S3 uploading works on a separate branch: https://gitlab.ddbuild.io/DataDog/benchmarking-platform-tools/-/jobs/2018290453 shows we can successfully fetch results from S3 for monitoring benchmark flakiness.