Skip to content

fix(agent-sre): measure benchmark duration with a monotonic clock - #4206

Merged
MohammadHaroonAbuomar merged 1 commit into
microsoft:mainfrom
1aifanatic:contrib/sre-benchmark-clock
Oct 2, 2026
Merged

MohammadHaroonAbuomar merged 1 commit into
microsoft:mainfrom
1aifanatic:contrib/sre-benchmark-clock

Conversation

@1aifanatic

@1aifanatic Naveen Chatlapalli (1aifanatic) commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Description

BenchmarkRunner measures elapsed duration using the wall clock. The existing test_latency_tracked fails on unchanged main under Windows because fast calls report zero duration; wall-clock adjustments can also distort timeout classification.

Use time.perf_counter() for duration measurements in both success and exception paths. Report timestamps continue using the wall clock.

Validation

Two deterministic cases cover a frozen wall clock, a successful call exceeding its timeout, and an exception. Both duration cases fail before the fix. Benchmark tests: 25 passed. Full package suite: 1,497 passed, 3 skipped.

All regressions were reproduced on origin/main 406ddd4d before the production change. Whitespace checks pass. Ruff passes on the changed files. Changed-line spelling passes with the existing API identifier Deduplicator allowed in the temporary check input.

Scope: the existing Agent-SRE source tree, which remains exercised by repository CI. Combined validation of these six independent SRE fixes: 1,505 passed, 3 skipped on Windows/Python 3.12.

Type of Change

  • Bug fix (non-breaking correction of existing behavior)

Package(s) Affected

  • agent-sre

Checklist

  • Changed code follows the project style checks; the exact validation scope is recorded above.
  • Regression tests or existing package tests verify the change, as detailed above.
  • The relevant test suites passed; commands, scope, and skips are recorded above.
  • Documentation is updated where needed; the behavior and compatibility implications are described above.
  • The Microsoft CLA check has passed for this contribution.

Attribution & Prior Art

  • Related prior contributions are credited below; this PR introduces no unattributed code from external projects.
  • External design influences, where applicable, are identified below.

Prior art / related projects: Uses the existing Agent-SRE implementation and standard library operations. No code was copied or adapted from an external project.

AI Assistance

I reviewed the specific changes in this diff before this PR was opened.

Codex assisted with implementation, regression tests, and local validation under my direction.

  • I reviewed this diff before submission; its behavior, rationale, and tradeoffs are documented above.
  • Tests and verification appropriate to this change were run, with limitations disclosed above.
  • Submission followed my direction and my review of these specific changes.
  • No AI-generated reviews of other contributors' PRs were posted as part of this contribution.

IP, Patents, and Licensing

  • This change uses the existing project implementation and ordinary software techniques; no proprietary or patent-specific technique is introduced.
  • Understanding or using this change requires no NDA or separate licensing agreement.
  • This contribution remains under the repository's MIT License and introduces no third-party source code or additional license terms.

Related Issues

Newly reproduced defect; the reproduction and regression coverage are described above.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions github-actions Bot added size/S Small PR (< 50 lines) tests agent-sre agent-sre package labels Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

PR Review Summary

Check Status Details
🔍 Code Review ⚠️ Missing No current-run comment
🛡️ Security Scan ⚠️ Missing No current-run comment
🔄 Breaking Changes ⚠️ Missing No current-run comment
📝 Docs Sync ⚠️ Missing No current-run comment
🧪 Test Coverage ⚠️ Missing No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AGT Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Oct 2, 2026
@MohammadHaroonAbuomar

Copy link
Copy Markdown
Collaborator

Verified in code and locally: a wall-clock step on main produced a negative latency and a PASSED result; perf_counter fixes it while report timestamps stay wall clock; both new tests fail on main. Sign-off is present and CI is approved and running.

Before I approve, two process items that apply to all nine PRs in this batch: please use the repository template with its Checklist, Attribution, AI Assistance and IP sections, and tick the attestations in your own words. The AI note says Codex prepared and submitted the changes at your request; CONTRIBUTING asks that no PR be submitted by an agent without a human reviewing the specific changes, so one sentence confirming you reviewed this diff before it was opened settles it. Once that and the gated checks are green I will approve and merge.

@1aifanatic

Copy link
Copy Markdown
Contributor Author

I reviewed the specific changes in this diff before the PR was opened. I have updated the description to use the repository template sections, completed the attestations with the validation scope and limitations, and clarified the AI-assistance note.

@MohammadHaroonAbuomar MohammadHaroonAbuomar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 8323314. Benchmark duration now uses time.monotonic; wall-clock jump reproduces the negative duration on main and not here; tests pass; ruff clean. Description uses the repository template with the attestations completed, and the author confirmed reviewing the diff before opening the PR. All 12 pull_request workflow runs on this head completed successfully. Commits are signed off.

@MohammadHaroonAbuomar
MohammadHaroonAbuomar merged commit a3ad6db into microsoft:main Oct 2, 2026
137 checks passed

@MohammadHaroonAbuomar MohammadHaroonAbuomar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 8323314. Benchmark duration now uses time.monotonic; wall-clock jump reproduces the negative duration on main and not here; tests pass; ruff clean. Description uses the repository template with the attestations completed, and the author confirmed reviewing the diff before opening the PR. All 12 pull_request workflow runs on this head completed successfully. Commits are signed off.

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

Labels

agent-sre agent-sre package needs-review:MEDIUM Contributor check flagged MEDIUM risk size/S Small PR (< 50 lines) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants