feat(tracer): add SpanKind/StatusCode API and unify resource attribute merging#17
Conversation
…e merging Three related improvements that were developed on feat/runtime-resource-attrs-and-span-status but never merged, now applied cleanly on top of main (which already has the ODR singleton fix from #16). 1. Span kind and status API Add SpanKind::{INTERNAL,SERVER,CLIENT,PRODUCER,CONSUMER} and StatusCode::{UNSET,OK,ERROR} constant namespaces. Add Span::setKind(), setStatus(), setError(), setOk(). The JSON serialiser now writes kind_ (defaulting to SERVER) and emits a status block only when statusCode_ != UNSET, per the OTLP spec. Move constructor and move-assignment updated to transfer the new fields. 2. buildResourceAttributes() — unified resource attribute merging Add buildResourceAttributes() to OtelDefaults.h. It merges runtime defaultResource() values with compile-time fallbacks (service.name, service.instance.id, host.name): runtime wins for any key that is set, compile-time fallback fills the rest. All three signal paths (traces, metrics, logs) now use this helper instead of three separate addResAttr() call sites. Replaces the all-or-nothing if/else approach introduced in #14 for metrics. 3. addResAttr() deprecated Mark the global addResAttr() helper [[deprecated]] pointing to buildResourceAttributes(). The private Span::addResAttr() copy is unchanged — it is only referenced by its own class. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 47 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR refactors OpenTelemetry resource attribute handling and extends the Span API with metadata support. A new ChangesSpan Metadata and Resource Attributes
Sequence DiagramsequenceDiagram
participant App as Application
participant Span as Span class
participant BuildRes as buildResourceAttributes
participant JSON as JSON Serialisation
App->>Span: setKind(SpanKind::CLIENT)
App->>Span: setStatus(StatusCode::OK)
App->>Span: end()
Span->>BuildRes: merge resource attrs
BuildRes->>JSON: populate resource.attributes
Span->>JSON: emit kind from kind_ field
Span->>JSON: conditionally emit status object
JSON-->>App: completed span OTLP
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR extends the tracer API to support explicit OTLP span kind and status, and centralizes resource attribute construction so runtime defaultResource() values are merged with compile-time fallbacks consistently across traces, metrics, and logs.
Changes:
- Add
SpanKind::*/StatusCode::*constants andSpan::{setKind,setStatus,setError,setOk}; update JSON span serialization to emitkindand optionally astatusblock. - Introduce
buildResourceAttributes()to merge runtime resource attributes with compile-time defaults (runtime wins per key). - Replace per-signal resource attribute logic with the shared helper and deprecate
addResAttr().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/OtelMetrics.cpp |
Switch metrics resource construction to buildResourceAttributes() (per-key merge instead of all-or-nothing). |
include/OtelTracer.h |
Add SpanKind/StatusCode APIs, serialize kind/status, and deprecate addResAttr(). |
include/OtelMetrics.h |
Update header comment to reflect removal of addResAttr() usage. |
include/OtelLogger.h |
Switch logs resource attributes to buildResourceAttributes(). |
include/OtelDefaults.h |
Add buildResourceAttributes() helper implementing runtime+fallback merging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Fix Span move-assignment TraceContext corruption
If the RHS span was the currently active context, calling end() on the
LHS would restore currentTraceContext() to the LHS parent's IDs, leaving
subsequent spans/logs linking to the wrong parent. Now the RHS active
state is checked before end(), and if it was active the context is
reinstalled after the field move.
2. Add tests for span kind and status serialisation
- default kind is SERVER (2)
- setKind(CLIENT) → kind field is 3
- status block absent when UNSET (default)
- setError("msg") → status.code=2 + status.message present
- setOk() → status.code=1, no status.message
3. Add tests for buildResourceAttributes() merge behaviour
- runtime service.name override appears in span resource attributes
- partial runtime override (only service.name set) still yields
compile-time fallback for service.instance.id and host.name
All 20 native tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
SpanKind::{INTERNAL,SERVER,CLIENT,PRODUCER,CONSUMER}andStatusCode::{UNSET,OK,ERROR}constant namespaces, plusSpan::setKind(),setStatus(),setError(),setOk(). The JSON serialiser now writes the actual kind (defaulting toSERVER) and emits astatusblock only when it has been explicitly set, per the OTLP spec.buildResourceAttributes(): new helper inOtelDefaults.hthat merges runtimedefaultResource()values with compile-time fallbacks. Runtime values win for any key that is set; compile-time fallbacks fill the rest. All three signal paths (traces, metrics, logs) now call this instead of three separateaddResAttr()sites. Also replaces the all-or-nothingif (!res.empty())approach introduced in feat(metrics): Use default resource labels for metrics. #14 for metrics.addResAttr()deprecated: marked[[deprecated]]pointing tobuildResourceAttributes().Background
This work was developed on
feat/runtime-resource-attrs-and-span-statusbut never merged. The ODR singleton fix from that branch was already landed in main via #16. This PR applies only the parts that are still missing, rebased cleanly onto current main with no conflicts.Test plan
pio test -e native)span.setKind(SpanKind::CLIENT)appears correctly in a collector backendspan.setError("something went wrong")emits astatusblock withcode=2andmessagedefaultResource().set("service.name", "my-device")overrides the compile-time default on all three signals🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
INTERNAL,SERVER,CLIENT,PRODUCER,CONSUMER) and status codes (UNSET,OK,ERROR).Refactor