Repository navigation
docs(#1094): unify SLO documentation - #1184
fullsend-ai-coder[bot] wants to merge 4 commits into
Conversation
Merge the content of the error-budgets research (SLOs-SLIs.md and users-expectations.md in packit/research) into the official Service Level Objectives page so there is a single authoritative document. The page now contains: - job-outcome definitions (success / failure / error) - the three SLOs with the existing official thresholds (SLO2 keeps "5 minutes after the Copr build has finished", not the research's 12 hours) - the SLIs per objective and how they are measured (Prometheus Pushgateway, histograms with buckets aligned to SLO thresholds, Grafana), checked against packit-service monitoring code - a rationale section explaining why each objective and indicator was chosen, based on the collected user expectations - guidance for other teams defining their own SLOs - a note that the page supersedes the research, which stays as historical background SLO headings keep their text, so existing anchors do not change. Note: the full site build (make import && yarn build) could not run in the sandbox because git clones fail on TLS verification. The page was compiled with @mdx-js/mdx 1.6.22 (the MDX version used by Docusaurus 2.4.1) instead. pre-commit could not fetch hook repos; check-merge-conflict and detect-private-key were run directly and passed; prettier excludes docs/. Closes #1094 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 8:18 AM UTC · Completed 8:29 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-5-5 · Effort: high · Cost: $3.90 |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 12s |
TomasTomecek
left a comment
There was a problem hiding this comment.
LGTM, I actually like the laid out casual tone
| The objectives below talk about jobs (builds and tests) ending with a status. | ||
| To make them unambiguous, we distinguish these outcomes: | ||
|
|
||
| - **success**: the job finished and everything is awesome |
|
<!-- fullsend:risk-assessment --> DetailsThis PR changes one documentation file (156 lines, which puts blast radius at medium) and touches no protected, security-sensitive, CI or dependency paths; the author is a bot, the file has had no recent churn or fix history, and the size fits the single-task issue it closes, giving 0.50x1.25 + 0.30x1.57 + 0.20x1.6 = 1.42, rounded to 1. Previous run<!-- fullsend:risk-assessment --> DetailsThis bot-authored PR changes only docs, edits a single file, touches no protected or security-sensitive paths and has no dependency or CI changes, so most signals score 1; the weighted total of 1.53 rounds up to 2 because the 145-line diff counts as a medium blast radius, the target doc was last changed over 180 days ago, and linked issue #1094 has been open for more than 180 days. Previous run (2)<!-- fullsend:risk-assessment --> DetailsThis bot PR changes one docs file and touches no protected, security, CI or dependency paths; it scores 1.53, rounded up to 2 (moderate), because of the medium blast radius of a 130-line change, a file untouched for more than 2.5 years, a linked issue open for more than 180 days, and retirement of the research page deferred to packit/research. Previous run (3)<!-- fullsend:risk-assessment --> DetailsDocs-only edit to one file by a known bot (130 lines), with no protected, security, CI or dependency paths, a file with no churn since January 2024, and a linked issue whose scope matches the PR (composite 0.5x1.25 + 0.3x1.57 + 0.2x1.8 = 1.46, rounded to 1). |
ReviewFindingsMedium / Low / Info
Previous run<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImVkZ2UtY2FzZSIsImZpbGUiOiJkb2NzL3NlcnZpY2UtbGV2ZWwtb2JqZWN0aXZlcy5tZCIsImlkIjoiZl8xMzVkMzk4NWY1ODI1ZTdmIiwibGluZSI6MTAyLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiJkb2NzL3NlcnZpY2UtbGV2ZWwtb2JqZWN0aXZlcy5tZCIsImlkIjoiZl80MGRhNjFjNzRiOWY5OTJiIiwibGluZSI6MTI0LCJhY3Rpb25hYmxlIjpmYWxzZX1dfQ== --> ReviewFindingsMedium / Low / Info
Next steps:
Previous run (2)<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6ImRvY3Mvc2VydmljZS1sZXZlbC1vYmplY3RpdmVzLm1kIiwiaWQiOiJmXzVmNTM5ZjY1MDQwMTQ0ZjIiLCJsaW5lIjo4NiwiYWN0aW9uYWJsZSI6dHJ1ZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoibG9naWMtZXJyb3IiLCJmaWxlIjoiZG9jcy9zZXJ2aWNlLWxldmVsLW9iamVjdGl2ZXMubWQiLCJpZCI6ImZfN2ZlZjgzNjUzMjhlMjA1NCIsImxpbmUiOjcwLCJhY3Rpb25hYmxlIjp0cnVlfV19 --> ReviewFindingsLow
Info
Labels: Docs-only SLO documentation change not tied to a specific integration, matching the area label on linked issue #1094. Next steps:
Previous run (3)<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6ImRvY3Mvc2VydmljZS1sZXZlbC1vYmplY3RpdmVzLm1kIiwiaWQiOiJmX2NiMjY5ODUzYmE2NjEyN2YiLCJsaW5lIjo3MCwiYWN0aW9uYWJsZSI6dHJ1ZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiZG9jLXN0eWxlIiwiZmlsZSI6ImRvY3Mvc2VydmljZS1sZXZlbC1vYmplY3RpdmVzLm1kIiwiaWQiOiJmX2UwNjY3ZGUyZjBjYTkxNTciLCJsaW5lIjoxMDEsImFjdGlvbmFibGUiOnRydWV9XX0= --> ReviewFindingsLow
On the reviewer note about SLO1's threshold: git history shows that commit 76caf5b (2023-03-22, "Update SLO1 definition") deliberately changed SLO1 from "a status update within 30 seconds" to "the first status update within 15 seconds". That came after the late-2022 change to 30s for all statuses. So the published 15-second value is the intended current threshold, and this PR is right to keep it. Labels: The PR only rewrites a documentation page (SLO docs). Next steps:
|
|
🤖 Finished Fix · ✅ Success · Started 8:30 AM UTC · Completed 8:34 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-5-5 · Effort: high · Cost: $0.64 |
- State the real 25-second limit of the SLO1 "no status" counter in the indicators table and note it is looser than the 15-second SLO1 limit. - Capitalize the first word after the colon in the rationale subheadings to match the Objectives headings. Note: pre-commit could not fetch hook repositories (TLS verification failure in the sandbox). check-merge-conflict and detect-private-key (pre-commit-hooks v5.0.0) were run directly and passed; the prettier hook excludes docs/. Addresses #1184 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🔧 Fix agent — iteration 1 (bot-triggered)Fixed both low-severity findings in docs/service-level-objectives.md. The SLO1 'no status' counter now states its real 25-second limit, and the rationale subheadings are capitalized. The repo has no test or lint suite, and a full site build needs network clones that fail in the sandbox. Instead, the page was compiled with @mdx-js/mdx 1.6.22 (the version Docusaurus 2.4.1 uses) without errors. pre-commit could not fetch its hook repos (TLS failure), so check-merge-conflict and detect-private-key were run directly and passed. The prettier hook excludes docs/. Project CI on the previous head was all passing. Fixed (3):
Tests: passed CI inspections (4):
Conflict update: skipped; strategy Decision points
Next steps:
|
|
🤖 Review · ❌ Terminated · Started 8:34 AM UTC · Ended 8:45 AM UTC Commit: |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 11s |
|
🤖 Finished Review · ✅ Success · Started 8:34 AM UTC · Completed 8:45 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-5-5 · Effort: high · Cost: $4.10 |
|
🤖 Finished Fix · ✅ Success · Started 8:46 AM UTC · Completed 8:50 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-5-5 · Effort: high · Cost: $0.75 |
Address the second round of review feedback on PR #1184: - Describe the SLO1 counter accurately: it counts PRs or commits whose first status took longer than 25 seconds, and neither SLO1 indicator sees events that never get a status. - Say that the histograms give the share of jobs that finished within the time threshold, whatever the outcome, so they approximate SLO2 and SLO3. Explain how error outcomes are handled for test runs (counted) and for builds (missing from the histogram). Verification: pre-commit could not fetch its hook repositories (TLS verification failure in the sandbox). The prettier hook excludes docs/. check-merge-conflict and detect-private-key were checked with equivalent pattern searches and passed. The page compiles with @mdx-js/mdx 1.6.22. Addresses #1184 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🔧 Fix agent — iteration 2 (bot-triggered)Addressed both low-severity logic-error findings in docs/service-level-objectives.md. The SLO1 counter is now described as counting first statuses that took longer than 25s, and the page notes that dropped events are not captured. The How we measure section now says the histograms measure only time, whatever the outcome, so they approximate SLO2/SLO3, and it explains how error outcomes are handled for test runs and builds. pre-commit could not fetch its hook repos (sandbox TLS failure). The applicable hooks were checked with equivalent pattern searches, and the page compiles with @mdx-js/mdx 1.6.22. Fixed (3):
Tests: passed CI inspections (4):
Conflict update: skipped; strategy Decision points
Next steps:
|
|
🤖 Review · ❌ Terminated · Started 8:51 AM UTC · Ended 9:02 AM UTC Commit: |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 11s |
|
🤖 Finished Review · ✅ Success · Started 8:51 AM UTC · Completed 9:01 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-5-5 · Effort: high · Cost: $4.03 |
|
🤖 Finished Fix · ✅ Success · Started 9:02 AM UTC · Completed 9:06 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-5-5 · Effort: high · Cost: $0.85 |
Address the third round of review feedback on PR #1184: - List the remaining ways the SLO2 histogram differs from the objective: Packit reports every unsuccessful Copr build as a failure, including Copr infrastructure failures, so those errors are measured as failures. Failed builds are not reported or measured when the SRPM build already failed or a newer build for the same target exists. - Add a note on SLO1 history: the limit was raised to 30 seconds for all statuses in late 2022 (packit-service#1776), and in March 2023 (commit 76caf5b) SLO1 was changed to the first status within 15 seconds. The published threshold is unchanged; maintainers should still confirm it matches current monitoring. Verification: pre-commit could not fetch its hook repositories (TLS verification failure in the sandbox), and the hook executables are not installed. The prettier hook excludes docs/. check-merge-conflict and detect-private-key were checked with equivalent pattern searches and passed. The page compiles with @mdx-js/mdx 1.6.22. Addresses #1184 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🔧 Fix agent — iteration 3 (bot-triggered)Addressed both review findings in docs/service-level-objectives.md. The SLO2 approximation list now says that Copr-side failures, including infrastructure failures, are measured as failures, and that failed builds are not measured when the SRPM build already failed or a newer build exists (checked against packit-service's CoprBuildEndHandler). A history note now explains the 2022 change of SLO1 to 30 seconds and the March 2023 change back to the first status within 15 seconds. A maintainer should still confirm the SLO1 definition and whether last_initial_status_time is an SLO1 indicator. The page compiles with MDX 1.6.22. pre-commit could not fetch its hook repos (sandbox TLS failure), so its checks were reproduced with equivalent pattern searches. All project CI checks pass. Fixed (2):
Disagreed (1):
Tests: passed CI inspections (4):
Conflict update: skipped; strategy Decision points
Next steps:
|
|
🤖 Review · ❌ Terminated · Started 9:07 AM UTC · Ended 9:19 AM UTC Commit: |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 14s |
Superseded by updated review
| first objective is about the first status only, with a strict time limit and a | ||
| high percentage. | ||
|
|
||
| In late 2022, the SLO1 limit was raised from 15 to 30 seconds to cover setting |
There was a problem hiding this comment.
[low] design-smell
The SLO1 history paragraph (raised to 30s in late 2022 in packit-service#1776, changed back to the first status within 15s in March 2023) is under 'Why these objectives and indicators', but it explains neither why SLO1 exists nor why 15s was chosen. The indicator table (line 70) also says the SLO1 counter uses a 25s limit, not the published 15s. So the official page shows a mismatch between the objective and how it is measured, and the PR body leaves this to maintainers.
Suggested fix: Have a maintainer confirm the SLO1 threshold against current monitoring. Then either make the text and the counter agree, or move the history note to the Background section so the rationale section only explains the choices.
| [what percentage of the measured jobs](https://prometheus.io/docs/practices/histograms/) | ||
| finished within the threshold, instead of relying on averages. | ||
|
|
||
| The histograms record only the time, whatever the outcome of the job. This means |
There was a problem hiding this comment.
[low] complexity-ratio
The 'How we measure' section ends with a detailed list (lines 95-110) of how the indicators differ from the objectives, taken from packit-service internals: histogram outcomes, how Copr infrastructure failures are reported, and when SRPM or newer-build cases skip reporting. These claims describe another repository's code, so they will go out of date whenever packit-service monitoring changes, and nothing here will catch it.
Suggested fix: Optional: consider shortening the list to one sentence saying the histograms only approximate SLO2 and SLO3, and track the detailed gaps in packit-service or packit/research.
|
🤖 Finished Review · ✅ Success · Started 9:07 AM UTC · Completed 9:19 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-5-5 · Effort: high · Cost: $4.30 |
Summary
Unifies Packit's SLO documentation into the official page
docs/service-level-objectives.md. The content of the error-budgets research (SLOs-SLIs.mdandusers-expectations.mdin packit/research) is merged in, so the website has one authoritative page instead of an official page plus a research page.The page now has:
packit_service/worker/monitoring.pyandjobs.pyin packit-service.Notes for reviewers
first_initial_status_timehistogram has a 15s bucket. However, theno_status_after_25_scounter uses 25s, and the week 47 2022 weekly post said SLO1 was raised from 15s to 30s (packit-service#1776). I did not change the published thresholds because the issue didn't ask for that.packit/researchthroughmake import, so it can't be retired here.research/monitoring/error-budgets/SLOs-SLIs.mdin packit/research should be changed to point to this page or marked as historical.Testing
make import && yarn build) could not run in the sandbox because git clones fail TLS verification. Instead, I compiled the page with@mdx-js/mdx@1.6.22(the MDX version Docusaurus 2.4.1 uses). The table, headings and admonition compiled without errors. All new links are external, and the image paths are unchanged. CI's test-deploy job runs the full build.check-merge-conflictanddetect-private-key(pre-commit-hooks v5.0.0) directly, and both passed. The prettier hook excludesdocs/.🤖 Generated with Claude Code
Closes #1094
Post-script verification
agent/1094-unify-slo-docs)c0e3687835a9f7786e74d7610b56d2c684879ec9..HEAD)