Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Enhances OTLP telemetry emitted by the gh-aw setup action and wires OTLP configuration into the Copilot Token Usage Optimizer workflow, with additional runtime metrics and improved export-failure tracking.
Changes:
- Extend OTLP attribute/value handling and enrich job conclusion spans with additional runtime/dashboard metrics.
- Track OTLP export failures via a persisted counter and surface that count on the workflow’s
conclusionjob span. - Update workflows to enable OTLP configuration/masking and adjust smoke workflow permissions for issue management.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/send_otlp_span.cjs | Adds richer numeric attribute encoding, export error tracking, agent runtime metrics parsing, and new conclusion-span attributes. |
| actions/setup/js/send_otlp_span.test.cjs | Adds/updates tests for new attribute encoding and telemetry attributes/error tracking behavior. |
| .github/workflows/smoke-project.lock.yml | Updates job permissions (issues) to support issue management in smoke workflows. |
| .github/workflows/copilot-token-optimizer.md | Adds OTLP observability configuration (endpoint/headers) to the workflow frontmatter. |
| .github/workflows/copilot-token-optimizer.lock.yml | Propagates OTLP env/secrets, masks headers, adds MCP OTel config, and uploads OTLP artifacts. |
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: 4
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 100/100✅ Excellent test quality
Test Classification DetailsView all 7 test classifications
HighlightsAll changes reflect a behaviorally-oriented test suite that tracks closely with the production changes:
Language Support
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd (new feature with test additions) and /zoom-out (attribute naming across the telemetry layer).
Key Themes
- Stale comment: The JSDoc block above
usageAttrsconstruction still describes the old "exclusive to agent span" contract, which was deliberately changed in this PR. Left as-is it will mislead future contributors. - Duplicate error count keys:
gh-aw.error_count(new, unconditional) andgh-aw.error.count(existing, conditional) now coexist with the same value when errors are present. One should be retired or the dual-write should be explained. - Weak test assertions for
recordOTLPExportError: Both newwriteSpyusages only assert.toHaveBeenCalled()— they don't pin the file path or the incremented value, so the tests would pass even if the function wrote to the wrong file with garbage content. - Missing
Infinity/NaNcoverage: The defensiveNumber.isFiniteguard inbuildAttris good but the corresponding test spec stops at1.25— edge cases for non-finite values are unspecified.
Positive Highlights
- ✅ Excellent defensive handling of non-finite/non-integer numbers in
buildAttr— avoids silent OTLP schema violations - ✅
endMscaptured once at function entry and reused throughout, eliminating clock skew between span events and span end time - ✅
readAgentRuntimeMetricsis well-isolated and self-contained; the log-parsing logic handles truncated JSON and empty lines cleanly - ✅ The new
gh-aw.run.statusnormalisation is a real quality-of-life improvement for dashboard queries - ✅ Good test coverage for the happy path of the new dashboard-metrics block
Verdict
No blocking issues — the changes are solid and well-tested overall. The inline comments point to a few sharpening opportunities (comment accuracy, attribute deduplication, test precision) worth addressing before merge.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 10.9M
…ypeScript error Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c0fcfada-8925-4b07-93c0-80fd9984bfac Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/23053d76-cf17-44b3-9004-f8d8eb8bc4dd Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/23053d76-cf17-44b3-9004-f8d8eb8bc4dd Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0ef0b634-382f-460c-8301-c65d7c13d124 Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
github-actions Bot
added a commit
that referenced
this pull request
May 7, 2026
- Fix tone issue in workflow-refactoring-patterns.md: remove gamified "+X points" scoring from Benefits section headings - Document mcp-gateway.opentelemetry.headers string-only format change from PR #30800 (breaking change with migration guidance) - Document aw_context caller metadata propagation for workflow_dispatch workflows (dispatch traceability via aw_info.json) - Document activation artifact now including aw_info.json alongside prompt.txt (earlier availability for prompt generation and logging) - Bump dev.md to v9.2, last updated 2026-05-07 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.