Skip to content

feat: add gh-aw.trigger.comment_id to setup and conclusion OTLP spans#29733

Merged
pelikhan merged 2 commits intomainfrom
copilot/add-gh-aw-trigger-comment-id-to-spans
May 2, 2026
Merged

feat: add gh-aw.trigger.comment_id to setup and conclusion OTLP spans#29733
pelikhan merged 2 commits intomainfrom
copilot/add-gh-aw-trigger-comment-id-to-spans

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 2, 2026

OTLP spans recorded enough context to find the triggering issue or PR but not the specific comment, forcing engineers to manually scroll through potentially hundreds of comments during trace investigations.

Changes

  • send_otlp_span.cjs: Extract awInfo.context.comment_id in both sendJobSetupSpan and sendJobConclusionSpan; conditionally emit as gh-aw.trigger.comment_id attribute (only when non-empty, consistent with item_type/item_number/trigger_label pattern)
  • JSDoc: Updated sendJobSetupSpan doc to include context.comment_idgh-aw.trigger.comment_id mapping
  • Tests: Added "emits gh-aw.trigger.comment_id when comment_id is non-empty" cases for both span functions; updated absence assertions in existing tests to also cover the new attribute key

New attribute follows the existing conditional-emit pattern:

const commentId = typeof awInfo.context?.comment_id === "string" ? awInfo.context.comment_id : "";
// ...
if (commentId) attributes.push(buildAttr("gh-aw.trigger.comment_id", commentId));

Copilot AI changed the title [WIP] Add OTel improvement: include gh-aw.trigger.comment_id in spans feat: add gh-aw.trigger.comment_id to setup and conclusion OTLP spans May 2, 2026
Copilot AI requested a review from pelikhan May 2, 2026 09:55
@pelikhan pelikhan marked this pull request as ready for review May 2, 2026 10:00
Copilot AI review requested due to automatic review settings May 2, 2026 10:00
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

This PR extends OTLP span metadata so setup and conclusion spans can include the triggering GitHub comment ID, making trace investigations link directly to the exact triggering comment instead of only the parent issue or PR.

Changes:

  • Added conditional emission of gh-aw.trigger.comment_id in both setup and conclusion OTLP spans from aw_info.context.comment_id.
  • Updated setup-span JSDoc to document the new context.comment_idgh-aw.trigger.comment_id mapping.
  • Added/updated tests to verify the new attribute is emitted when present and omitted when absent.
Show a summary per file
File Description
actions/setup/js/send_otlp_span.cjs Adds comment_id extraction and emits it as a span attribute in setup and conclusion span builders.
actions/setup/js/send_otlp_span.test.cjs Adds assertions covering presence/absence of the new OTLP attribute in both span test suites.

Copilot's findings

Tip

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

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

@pelikhan pelikhan merged commit 44fb9c0 into main May 2, 2026
22 of 23 checks passed
@pelikhan pelikhan deleted the copilot/add-gh-aw-trigger-comment-id-to-spans branch May 2, 2026 10:03
@github-actions github-actions Bot mentioned this pull request May 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 2, 2026

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

Excellent

Metric Value
New/modified tests analyzed 2 new + 3 assertion additions to existing tests
✅ Design tests (behavioral contracts) 2 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 2 (100%) — negative-absence assertions paired with each positive test
Duplicate test clusters 0
Test inflation detected ⚠️ Yes — 44 test lines added vs. 8 production lines (5.5:1 ratio)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Notes
"emits gh-aw.trigger.comment_id when comment_id is non-empty" (setup) send_otlp_span.test.cjs ✅ Design Verifies gh-aw.trigger.comment_id attribute is present in setup OTLP span
"emits gh-aw.trigger.comment_id when comment_id is non-empty" (conclusion) send_otlp_span.test.cjs ✅ Design Verifies gh-aw.trigger.comment_id attribute is present in conclusion OTLP span
expect(keys).not.toContain("gh-aw.trigger.comment_id") additions (×3) send_otlp_span.test.cjs ✅ Design Negative assertions added to existing tests ensuring attribute is absent when comment_id is missing

Flagged Tests — Requires Review

⚠️ Test Inflation (send_otlp_span.test.cjs)

Classification: Not a failing condition — vitest's OTLP test structure is inherently verbose
Observation: 44 test lines were added vs. 8 production lines (5.5:1 ratio, threshold 2:1). This is expected in telemetry tests: each test requires mock setup (vi.fn(), vi.stubGlobal, readFileSpy), invocation, JSON body parsing, and a deep attribute assertion — all necessary boilerplate. The verbosity reflects real behavioral coverage, not padding.
Action required: None — no penalty applied in spirit, though the formula deducted 10 points mechanically.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 0 tests (no Go test files changed)
  • 🟨 JavaScript (*.test.cjs): 2 new test functions + 3 assertion additions (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). Both new tests verify observable OTLP span attributes — behavioral contracts that directly validate the PR's feature. Mocking is limited to external I/O (HTTP fetch and filesystem reads), which is appropriate. The paired negative assertions (not.toContain) confirm the attribute is conditionally emitted only when comment_id is present.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §25249431955

🧪 Test quality analysis by Test Quality Sentinel · ● 694.7K ·

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.

✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). Both new tests verify observable OTLP span attributes as behavioral contracts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[otel-advisor] OTel improvement: add gh-aw.trigger.comment_id to setup and conclusion spans

3 participants