Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Enhances the Smoke OTEL workflow to validate Datadog alongside Sentry and Grafana.
Changes:
- Adds Datadog OTLP and MCP configuration to the smoke workflow.
- Expands smoke-test instructions, status logic, and reporting table to include Datadog.
- Updates generated lock workflows and shared Datadog/OTLP imports.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/smoke-otel-backends.md |
Adds Datadog validation steps and reporting guidance. |
.github/workflows/smoke-otel-backends.lock.yml |
Regenerates the runnable workflow with Datadog secrets, MCP server config, and allowed domains. |
.github/workflows/shared/otlp-sentry-grafana-datadog.md |
Adds shared OTLP configuration for Sentry, Grafana, and Datadog. |
.github/workflows/shared/mcp/datadog.md |
Updates Datadog MCP server configuration and allowed tools. |
.github/workflows/mcp-inspector.lock.yml |
Regenerates inspector lock output for updated Datadog MCP configuration. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 3
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /zoom-out — this PR extends an existing two-backend pattern (Sentry + Grafana) to a third backend (Datadog), so the key question is whether the new pieces fit consistently with what's already there.
Key Themes
- Docs/config mismatch in
datadog.md: The prose section says "Allowed tools in this shared import" and lists 6 tools, but theallowedYAML config only contains 2. The documentation overstates the permitted tool surface. - Dead secret entry:
DD_SITE (optional)appears in the secrets list but is never interpolated into any config, so its purpose is unclear. - Underspecified verdict for
inconclusive: Datadog introduces a third status value (inconclusive) that has no explicit mapping in the Step 5 final verdict rule. - Backend-encoded filename:
otlp-sentry-grafana-datadog.mdwill need renaming if a fourth backend is added; a generic name would be more durable.
Positive Highlights
- ✅ Clean, consistent extension of the Sentry/Grafana pattern — the Step 4 structure mirrors the existing steps well.
- ✅ The
inconclusivestatus concept is well-reasoned (ingestion delay vs broken path) and the fallback 24-hour query is a practical heuristic. - ✅ Good use of the
?toolsets=coreURL parameter to keep the Datadog tool surface narrow. - ✅ The updated
datadog.mdswitches to the official Datadog MCP endpoint and corrects the secret name (DD_APP_KEYvs the oldDD_APPLICATION_KEY).
Verdict
No merge-blocking issues — the three inline comments are worth addressing before the workflow runs in production, especially the inconclusive verdict gap and the doc/config mismatch in datadog.md.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 15.6M
This comment has been minimized.
This comment has been minimized.
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot Address all remaining review comments by github-actions. Then recompile workflow. |
|
✅ smoke-ci: safeoutputs CLI comment + comment-memory run (26033889688)
|
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Done in |
|
@copilot |
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Updated in |
|
@copilot Address datadog issues found in run https://github.com/github/gh-aw/actions/runs/26035293666 |
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Addressed in |
|
|
chore: enhance smoke test workflow to include datadog validation
Summary
Extends the OTEL smoke test infrastructure to validate Datadog as a
third observability backend alongside Sentry and Grafana. Adds a
dedicated Datadog MCP query step to the smoke workflow, wires in the
official Datadog MCP server, and introduces a new shared
otlp-backends.mdimport that consolidates OTLP export configurationfor all three backends.
What changed and why
.github/workflows/smoke-otel-backends.md(modified — high impact)and Grafana configs.
shared/otlp.mdto the newshared/otlp-backends.mdimport, which fans out OTLP export to all three backends in one step.
datadog_statusresult.datadog_statusinto the final pass/fail verdict, so aDatadog failure now gates the smoke run.
.github/workflows/shared/otlp-backends.md(added — high impact)Grafana, and Datadog in a single, reusable block.
secret, replacing the narrower single-backend
shared/otlp.mdimport..github/workflows/shared/mcp/datadog.md(modified — medium impact)coretoolset pinned.|| secrets.DD_APP_KEYfallback to align with theDD_APP_KEY/DD_APPLICATION_KEYdual-naming convention used across the repo.search_datadog_spansandget_datadog_traceto the allowedtool list.
tool surface.
Key commits
bcd3fc99383f6c43f0DD_APPLICATION_KEY5ae9c264756d148e63DD_APP_KEY; recompile6fc195679Risk & compatibility
DD_APP_KEY/DD_APPLICATION_KEYmust be present for the Datadog step to passdatadog_statusis now part of the verdict — workflows without the Datadog secret configured will fail the smoke runReviewer focus areas
shared/otlp-backends.md— verify all three backend endpoint/authsecret names are correct for the target environment.
smoke-otel-backends.mdStep 4 — confirm the Datadog query logicand the way
datadog_statusis folded into the verdict matches thepattern used for
sentry_status/grafana_status.mcp/datadog.mdfallback — confirm the|| secrets.DD_APP_KEYprecedence order is intentional (prefer
DD_APPLICATION_KEY, fallback to
DD_APP_KEY).