Skip to content

ci: gate notify-slo-breaches like check-slo-breaches - #4165

Open
igoragoli wants to merge 1 commit into
masterfrom
augusto/php-notify-slo-match-check-gate
Open

ci: gate notify-slo-breaches like check-slo-breaches#4165
igoragoli wants to merge 1 commit into
masterfrom
augusto/php-notify-slo-match-check-gate

Conversation

@igoragoli

@igoragoli igoragoli commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Description

On this pipeline, check-slo-breaches was skipped due to it being a nightly benchmark pipeline, but notify-slo-breaches ran and incorrectly notified a failure.

notify-slo-breaches should not run on nightly benchmarks, and this PR disables it.

  • Add when: manual and allow_failure: true to the fallthrough rule of .notify-slo-breaches-rules, matching .macrobenchmarks-rules.
    • The fallthrough carried no when:, so the job inherited when: always from the template and ran even when its shared if: condition was false.
    • On a scheduled master pipeline with NIGHTLY_BENCHMARKS unset, macrobenchmarks and check-slo-breaches fall through to manual and are never triggered. notify-slo-breaches ran anyway.
    • The notifier reads no exit code: it tests for the fail-on-breach.status.check-completed marker file, so a check that never ran looks the same as one that failed.
  • Leave the first rule untouched, so when: always still holds on master and release branches, where a check failure means a breach was detected.

Nightly pipelines now send no notification, which is intended: there is no check result to report.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@igoragoli
igoragoli requested review from a team as code owners September 7, 2026 14:47
@igoragoli igoragoli added the AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos label Sep 7, 2026
The fallthrough rule for notify-slo-breaches carried no `when:`, so the job
inherited `when: always` from the template and ran in pipelines where
check-slo-breaches itself fell through to `when: manual` and was never
triggered. Since the notifier tests for the check-completed marker file rather
than an exit code, a check that never ran looks the same as one that failed,
and scheduled master pipelines without NIGHTLY_BENCHMARKS alerted "SLO checks
failed to complete" with nothing wrong.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@igoragoli
igoragoli force-pushed the augusto/php-notify-slo-match-check-gate branch from 23a4644 to 1713f9c Compare September 7, 2026 14:49
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Sep 7, 2026

Copy link
Copy Markdown

Pipelines  Tests

Unblock PR with BitsAI

⚠️ Warnings

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 40 Pipeline jobs failed

DataDog/apm-reliability/dd-trace-php | check libxml2 version — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

DataDog/apm-reliability/dd-trace-php | test_extension_ci: [7.2, valgrind] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

DataDog/apm-reliability/dd-trace-php | test_extension_ci: [8.3] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

View all 40 failed jobs.

ℹ️ Info

No other issues found (see more)

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 54.99% (-0.06%)

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 1713f9c | Docs | View more details | Give us feedback!

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

Labels

AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant