Skip to content

Conversation

@brettlangdon
Copy link
Member

Description

Only run SLO check if the benchmark jobs all succeeded.

Only run the reporting if the SLO check job ran.

Right now if a benchmarks fails (programming error, or infra issues, etc), we still run the SLO check, which we know will fail because we don't report all benchmark results, then we also run the benchmark reporter which will succeed. If you then go back and re-run the failed benchmark job (assuming flaky infra, for example), then you have to know to manually re-run the SLO check and the reporting job as well).

This should simplify and clarify some of it a little bit.

Testing

Risks

Additional Notes

Only run SLO check if the benchmark jobs all succeeded.

Only run the reporting if the SLO check job ran.
@brettlangdon brettlangdon requested a review from a team as a code owner November 19, 2025 15:00
@brettlangdon brettlangdon added the changelog/no-changelog A changelog entry is not required for this PR. label Nov 19, 2025
@brettlangdon brettlangdon requested a review from a team as a code owner November 19, 2025 15:00
@github-actions
Copy link
Contributor

CODEOWNERS have been resolved as:

.gitlab/benchmarks/microbenchmarks.yml                                  @DataDog/python-guild @DataDog/apm-core-python

@brettlangdon brettlangdon enabled auto-merge (squash) November 19, 2025 15:01
@github-actions
Copy link
Contributor

Bootstrap import analysis

Comparison of import times between this PR and base.

Summary

The average import time from this PR is: 261 ± 5 ms.

The average import time from base is: 269 ± 5 ms.

The import time difference between this PR and base is: -8.0 ± 0.2 ms.

Import time breakdown

The following import paths have shrunk:

ddtrace.auto 7.014 ms (2.69%)
ddtrace.bootstrap.sitecustomize 4.187 ms (1.60%)
ddtrace.bootstrap.preload 4.187 ms (1.60%)
ddtrace.internal.settings.dynamic_instrumentation 0.698 ms (0.27%)
ddtrace.internal.remoteconfig.worker 0.684 ms (0.26%)
ddtrace.internal.remoteconfig.client 0.656 ms (0.25%)
ddtrace 2.826 ms (1.08%)
ddtrace.trace 0.746 ms (0.29%)
ddtrace._logger 0.693 ms (0.27%)
ddtrace.internal.telemetry 0.693 ms (0.27%)
ddtrace.internal.telemetry.writer 0.693 ms (0.27%)
ddtrace.internal.utils.version 0.693 ms (0.27%)
ddtrace.internal._unpatched 0.032 ms (0.01%)
json 0.032 ms (0.01%)
json.decoder 0.032 ms (0.01%)
re 0.032 ms (0.01%)
enum 0.032 ms (0.01%)
types 0.032 ms (0.01%)

@igoragoli igoragoli self-requested a review November 19, 2025 15:48
Copy link
Contributor

@igoragoli igoragoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from APM DCS Performance.

Summarizing a discussion I had with @brettlangdon for future readers:

when: always is useful on other other repos where it's possible to merge a PR/release with a failing benchmarking job, so we rely on the SLO check for a general gating mechanism.

On the other hand, on dd-trace-py this is not possible, so if a microbenchmarking job fails, the SLO check is not necessary since we are not going to be able to merge the PR anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants