Skip to content

Add logging improvements to AGENTS.md#4825

Merged
openshift-merge-bot[bot] merged 1 commit intomainfrom
log-improvement-documentation
Apr 11, 2026
Merged

Add logging improvements to AGENTS.md#4825
openshift-merge-bot[bot] merged 1 commit intomainfrom
log-improvement-documentation

Conversation

@mgahagan73
Copy link
Copy Markdown
Collaborator

What

Add recently logging improvement suggestions to AGENTS.md

Why

Reduce log clutter by improving logging hygene and implementing a delta logging model.

Copilot AI review requested due to automatic review settings April 9, 2026 20:06
@openshift-ci openshift-ci bot requested review from bennerv and deads2k April 9, 2026 20:06
@openshift-ci openshift-ci bot added the approved label Apr 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the E2E testing guidelines in test/AGENTS.md to reduce noisy logs in polling-style tests by recommending delta-only, expectation-focused logging patterns.

Changes:

  • Add a new “Logging in Eventually/Polling Tests” section with guidance for delta-only logging and actionable failure output.
  • Reference the existing eventuallyVerify helper as a baseline pattern for polling verifications.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/AGENTS.md

4. **Consider dumping targeted state on failure:** When a polling loop times out, it is strongly recommended to dump the status of the specific resources you were directly polling — e.g. if you were waiting for Machines to upgrade, dump those Machine statuses. Keep this narrowly scoped to avoid log clutter; broad cluster diagnostics should be left to `oc adm inspect` or equivalent artifact collection. The goal is that the most immediately relevant context appears inline next to the failure message.

5. **Use `eventuallyVerify` or equivalent patterns:** The `eventuallyVerify` helper in `e2e/cluster_pullsecret.go` implements delta-only logging by tracking the previous error string. This pattern (based on HyperShift's `EventuallyObject` in [test/e2e/util/eventually.go](https://github.com/openshift/hypershift/blob/main/test/e2e/util/eventually.go)) should be used as the baseline for all polling-style verifications. Consider also logging a timestamp for when the `eventuallyVerify` or similar polling operation completes successfully if the verifier you are using does not already implement this function.
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The reference to eventuallyVerify points to e2e/cluster_pullsecret.go, but the file lives under test/e2e/cluster_pullsecret.go (and elsewhere in this doc you use test/e2e/...). Consider updating the path for consistency and to make the link/location unambiguous.

Suggested change
5. **Use `eventuallyVerify` or equivalent patterns:** The `eventuallyVerify` helper in `e2e/cluster_pullsecret.go` implements delta-only logging by tracking the previous error string. This pattern (based on HyperShift's `EventuallyObject` in [test/e2e/util/eventually.go](https://github.com/openshift/hypershift/blob/main/test/e2e/util/eventually.go)) should be used as the baseline for all polling-style verifications. Consider also logging a timestamp for when the `eventuallyVerify` or similar polling operation completes successfully if the verifier you are using does not already implement this function.
5. **Use `eventuallyVerify` or equivalent patterns:** The `eventuallyVerify` helper in `test/e2e/cluster_pullsecret.go` implements delta-only logging by tracking the previous error string. This pattern (based on HyperShift's `EventuallyObject` in [test/e2e/util/eventually.go](https://github.com/openshift/hypershift/blob/main/test/e2e/util/eventually.go)) should be used as the baseline for all polling-style verifications. Consider also logging a timestamp for when the `eventuallyVerify` or similar polling operation completes successfully if the verifier you are using does not already implement this function.

Copilot uses AI. Check for mistakes.
@raelga
Copy link
Copy Markdown
Collaborator

raelga commented Apr 11, 2026

/lgtm

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 11, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mgahagan73, raelga

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 4d95d60 into main Apr 11, 2026
20 checks passed
@openshift-merge-bot openshift-merge-bot bot deleted the log-improvement-documentation branch April 11, 2026 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants