chore: Update otel smoke tests#32598
Conversation
|
@copilot Recompile workflow |
|
✅ smoke-ci: safeoutputs CLI comment + comment-memory run (25958878668)
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
Done — ran a full recompile across all 228 workflows (0 errors, 0 warnings). The |
There was a problem hiding this comment.
Pull request overview
Removes the obsolete generic OTEL smoke test workflow and refines the OTEL backends smoke test’s failure-reporting instructions to produce more actionable, structured results.
Changes:
- Deleted the legacy
Smoke OTELworkflow definition (smoke-otel.md) and its compiled lock workflow (smoke-otel.lock.yml). - Updated
smoke-otel-backends.mdoutput requirements to use a per-backend result table plus a dedicated## Failure Analysissection.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/smoke-otel.md |
Removed obsolete generic OTEL smoke test workflow source. |
.github/workflows/smoke-otel.lock.yml |
Removed compiled lock workflow for the obsolete Smoke OTEL workflow. |
.github/workflows/smoke-otel-backends.md |
Updated issue output format to a backend matrix table and structured failure analysis guidance. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd and /zoom-out for this tests-only PR that updates workflow prompt instructions and removes a redundant workflow.
Key Themes
-
Coverage gap from deletion:
smoke-otel.mdtested the local OTLP infrastructure — env vars set, JSONL mirror created, spans written, zero export errors.smoke-otel-backends.mdtests remote backend reachability. These are complementary layers; removing the local smoke test means a misconfigured OTLP env (e.g. missingGH_AW_OTLP_ENDPOINTS) could silently pass the backends test (the agent would just report inconclusive). Worth confirming the local checks are now implicitly covered by Step 1 ofsmoke-otel-backends.md, or adding a note that this coverage is intentionally dropped. -
Table cell notation:
[x]/[ ]in Markdown table cells renders as literal text, not checkboxes. Emoji (✅/❌) would be more readable. (Inline comment added.) -
Agent ordering under budget pressure: With
max-continuations: 1, the agent may deprioritise Grafana if Sentry investigation is expensive. The new "do not stop after first failure" instruction is good, but explicit step ordering would reinforce it. (Inline comment added.)
Positive Highlights
- ✅ The structured table + dedicated
## Failure Analysissubsections are a meaningful improvement over the old flat checklist — much easier for on-call engineers to parse at a glance. - ✅ Requiring the root cause category (write path / read path / auth / config / propagation / backend itself) is excellent — it eliminates the most common "I don't know where to start" friction in failure triage.
- ✅ Deleting the obsolete
smoke-otel.lock.ymlalongsidesmoke-otel.mdis clean housekeeping.
Verdict
LGTM with the minor nits above. The deletion of smoke-otel.md deserves a quick confirmation that its local-infrastructure coverage is intentionally retired or absorbed elsewhere.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 6M
Uh oh!
There was an error while loading. Please reload this page.