Skip to content

chore: enhance otel smoke tests#32601

Merged
mnkiefer merged 2 commits into
mainfrom
smoke-otel-2
May 16, 2026
Merged

chore: enhance otel smoke tests#32601
mnkiefer merged 2 commits into
mainfrom
smoke-otel-2

Conversation

@mnkiefer
Copy link
Copy Markdown
Collaborator

  • Updates the smoke test workflow documentation for OTEL (OpenTelemetry) backends to clarify the order of checks, improve reporting consistency, and specify the required format for status tables and evidence.

@mnkiefer mnkiefer self-assigned this May 16, 2026
@mnkiefer mnkiefer marked this pull request as ready for review May 16, 2026 10:19
Copilot AI review requested due to automatic review settings May 16, 2026 10:19
@mnkiefer
Copy link
Copy Markdown
Collaborator Author

@copilot Recompile workflow

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

Documentation-only update clarifying the smoke test workflow for OTEL backends.

Changes:

  • Adds explicit note that Step 1 covers local OTEL infrastructure checks before remote backend queries.
  • Requires completing all three steps (Local, Sentry, Grafana) in order and reporting both backends regardless of failure.
  • Updates the status table format to include a Local OTLP row and switches status cells from [x]/[ ] to emoji symbols (✅/❌/⚪/—).
Show a summary per file
File Description
.github/workflows/smoke-otel-backends.md Clarifies Step 1 scope, mandates backend ordering/reporting, and updates the required status table format and symbols.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@jobayer-4
Copy link
Copy Markdown

#32279

@github-actions
Copy link
Copy Markdown
Contributor

✅ smoke-ci: safeoutputs CLI comment + comment-memory run (25959447654)

Generated by 🧪 Smoke CI for issue #32601 ·

@github-actions
Copy link
Copy Markdown
Contributor

Comment Memory

CI lights the path
Green checks bloom at dawn
Quiet bots still sing

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Generated by 🧪 Smoke CI for issue #32601 ·

@github-actions github-actions Bot mentioned this pull request May 16, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /grill-with-docs — this is a documentation-only change to the smoke test workflow spec, so the focus is on terminology consistency and self-documenting clarity.

Key Themes

  • Ordering and completeness rules (lines 74–75): the two new rules are clear and actionable for the AI agent — explicit instruction not to skip Grafana if Sentry fails is exactly the kind of guard rail this workflow needs.
  • Status emoji upgrade: replacing [x]/[ ] with /// improves scannability. Minor concern: for inconclusive can look visually like "empty/not done" — see inline comment.
  • Local OTLP row: adding a dedicated Local OTLP row is a meaningful improvement because it surfaces the local emission path as a first-class check rather than an implicit precondition. The cells appropriately acknowledge that local OTLP has no remote read path.

Positive Highlights

  • ✅ The Step 1 introductory paragraph (line 53) neatly summarises what local checks cover before any remote queries, which helps orient the AI agent.
  • ✅ The rule "Do not skip Grafana because Sentry failed or consumed time" directly addresses a real AI agent failure mode (abandoning later steps when earlier ones consume budget).
  • ✅ The FAIL verdict in the example table (Grafana | ✅ | ❌ | ✅ | ✅ | ❌) correctly demonstrates that a failed write export drives the overall row to ❌ even when read succeeds — that's the right semantic.

Verdict

Approving with two minor suggestions (inline): symbol choice for inconclusive, and a note clarifying how the shared column headers map to the local OTLP row.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 5.2M

Comment thread .github/workflows/smoke-otel-backends.md Outdated
Comment thread .github/workflows/smoke-otel-backends.md
Copy link
Copy Markdown
Contributor

Copilot AI commented May 16, 2026

@copilot Recompile workflow

Ran make recompile on the latest commit (588dd11). Workflow compilation completed cleanly and produced no additional file changes.

@mnkiefer
Copy link
Copy Markdown
Collaborator Author

@copilot Address all review comments from github-actions

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

@copilot review all comments and address the remaining inline review feedback, starting with the Local OTLP mapping note. Please summarize any blockers that still remain afterward.

Generated by 👨‍🍳 PR Sous Chef ·

Copy link
Copy Markdown
Contributor

Copilot AI commented May 16, 2026

@copilot Address all review comments from github-actions

Done in 73abb40. Addressed the github-actions review feedback by switching inconclusive to 🔶 and adding explicit Local OTLP column mapping guidance in the output section.

@mnkiefer mnkiefer merged commit 7ebda5b into main May 16, 2026
@mnkiefer mnkiefer deleted the smoke-otel-2 branch May 16, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants