Skip to content

Add kindling race keys - #31

Merged
myleshorton merged 2 commits into
mainfrom
fisk/kindling-keys
Oct 11, 2026
Merged

myleshorton merged 2 commits into
mainfrom
fisk/kindling-keys

Conversation

@myleshorton

@myleshorton myleshorton commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Adds attribute keys for kindling races, for getlantern/radiance#666. That PR records on each request's span which bootstrap transport carried it (domainfront, smart, amp, soar) and what happened to the others, using kindling's new WithRaceObserver (getlantern/kindling#57).

  • kindling.winner, plus attempt-event keys kindling.transport, .outcome, .status, .elapsed_ms and .error.
  • KindlingTransportAttr(transport, key) builds the per-transport span attributes, for example kindling.smart.outcome and kindling.smart.status. These let SigNoz filter and group by one transport's outcome, such as "IR clients where kindling.smart.status = 403". Transport names come from the caller's kindling configuration, so cardinality is bounded.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BW8kRuU5CnDvuK8jha5ear

Summary by CodeRabbit

  • New Features
    • Added standardized telemetry attributes for race winners, transports, outcomes, HTTP status codes, elapsed time, and errors.
    • Added support for transport-specific attribute names, helping distinguish telemetry recorded across different transports.

radiance records which kindling transport carried each request and how the
others fared (getlantern/radiance#666). KindlingTransportAttr builds the
per-transport form (kindling.smart.outcome) so spans carry every
transport's outcome as groupable attributes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BW8kRuU5CnDvuK8jha5ear
Copilot AI balanced review requested due to automatic review settings October 11, 2026 13:19
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 51 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9f587b88-540b-43bf-955d-c8d3d81b7798

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca9f95 and 9d74bcf.


📒 Files selected for processing (1)
  • custom.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 19b26898-d5c9-4c70-b811-77e0535cbaa5


📥 Commits

Reviewing files that changed from the base of the PR and between 6897603 and 0ca9f95.



📒 Files selected for processing (2)
  • custom.go
  • kindling_test.go


Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.




📝 Walkthrough
📝 Walkthrough

Walkthrough

The change adds public Kindling attribute keys for race and transport results, plus a helper that creates transport-scoped keys. A test checks the helper’s output for outcome, status, and elapsed-milliseconds attributes.

Changes

Kindling attributes

Layer / File(s) Summary
Attribute keys and transport-scoped helper
semconv/custom.go, kindling_test.go
Adds Kindling attribute keys and KindlingTransportAttr. The test checks generated keys for the "smart" transport.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Feature

Suggested reviewers: jay-418



Merge Risk: ⚪ Minimal · up to 0ca9f

This adds new telemetry attribute names and a helper for building transport-scoped names. It does not change how existing telemetry is produced or exported, so merge risk is minimal.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 0ca9f

The change adds telemetry key names and a side-effect-free key-construction helper. It does not itself collect or export telemetry, change access controls, or introduce shared-state transitions. No material security risk was identified in the inspected changes.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated effect of the new API is attribute-name construction within a calling process. The inspected additions do not create a network entrypoint, privileged operation, or telemetry-export path; downstream adoption is outside the verified scope.

Trust Boundaries and Controls

  • observed — Caller-supplied strings flow directly into a returned key, not into an execution or export sink. KindlingErrorKey names an attribute but does not capture or redact error data. Transport provenance and value handling must therefore be assessed in downstream instrumentation rather than inferred from these declarations.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding Kindling race attribute keys and related support.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.



✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR



🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR



  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Duplicate transport names can silently overwrite race outcomes in flattened span attributes.

1 open finding
What changed in this PR

Adds canonical Kindling race telemetry keys and a helper for transport-scoped span attributes.

Changes:

  • Defines Kindling winner and attempt attribute keys.
  • Adds and tests transport-scoped key generation.
File Description
custom.go Adds Kindling keys and transport-scoping helper.
kindling_test.go Tests generated transport attribute names.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread custom.go
Review on #31 (Copilot).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BW8kRuU5CnDvuK8jha5ear

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused changes are consistent with repository conventions and adequately tested.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@myleshorton
myleshorton merged commit f5e57ec into main Oct 11, 2026
2 checks passed
myleshorton added a commit to getlantern/radiance that referenced this pull request Oct 11, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BW8kRuU5CnDvuK8jha5ear
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.

2 participants