code review comments.

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
This commit is contained in:
Pratik Mankawde
2026-07-06 20:49:51 +01:00
parent e969b0c6ed
commit 7998d01dc9
4 changed files with 34 additions and 15 deletions

View File

@@ -278,9 +278,11 @@ See [Phase4_taskList.md](./Phase4_taskList.md) for full task details.
**Objective**: Wire `TraceContextPropagator` for P2P messages (proposals,
validations) to enable true distributed tracing between nodes.
**Status**: Design documented, NOT implemented. Protobuf fields (field 1001)
and `TraceContextPropagator` free functions exist. Wiring deferred until Phase 4a is
validated in a multi-node environment.
**Status**: Partially implemented. Send-side injection (proposals and
validations) and receive-side extraction (`consensus.{proposal,validation}.
receive` spans parented on the sender's context) are wired in Phase 4a.
Remaining Phase 4b work: relay spans in `share(RCLCxPeerPos)` and multi-node
validation of the propagation path.
**Prerequisites**: Phase 4a complete and validated.

View File

@@ -95,10 +95,16 @@
- Creates `consensus.proposal.receive` span
- Sets `trusted` attribute (bool)
**Not implemented** (deferred to Phase 4b — cross-node propagation):
**Done here** (cross-node propagation, send + receive):
- `consensus.proposal.relay` span in `share(RCLCxPeerPos)` — requires trace context injection
- Trace context injection/extraction for `TMProposeSet::trace_context`
- Trace context injection for `TMProposeSet::trace_context` in `propose()`
- Receive-side extraction in `PeerImp::onMessage(TMProposeSet)` via
`telemetry::proposalReceiveSpan()` (parents the receive span on the
sender's context when a valid `trace_context` is present)
**Not implemented** (deferred to Phase 4b):
- `consensus.proposal.relay` span in `share(RCLCxPeerPos)`
**Key modified files**:

View File

@@ -144,12 +144,12 @@ datasources:
tag: close_time_correct
operator: "="
scope: span
type: dynamic
type: static
- id: consensus-state
tag: consensus_state
operator: "="
scope: span
type: dynamic
type: static
- id: consensus-close-resolution
tag: close_resolution_ms
operator: "="

View File

@@ -1260,8 +1260,25 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
telemetry::SpanContext const* const link =
prevRoundSpanContext_.isValid() ? &prevRoundSpanContext_ : nullptr;
if (strategy == "deterministic")
if (strategy == "attribute")
{
// Non-deterministic strategy: each node gets a random trace_id,
// correlated via the consensus_ledger_id attribute rather than a
// shared trace_id. Still attach a follows-from link to the prior
// round so consecutive rounds stay navigable. linkedSpan is not
// TraceCategory-aware, so gate it explicitly to match the gating
// of the hashSpan/span factories used below.
if (link != nullptr && app_.getTelemetry().shouldTraceConsensus())
roundSpan_.emplace(telemetry::SpanGuard::linkedSpan(cs::round, *link));
else
roundSpan_.emplace(
telemetry::SpanGuard::span(
telemetry::TraceCategory::Consensus, telemetry::seg::consensus, cs::op::round));
}
else
{
// "deterministic" (the default): derive the trace_id from the previous
// ledger hash so all validators tracing the same round share one trace.
roundSpan_.emplace(
telemetry::SpanGuard::hashSpan(
telemetry::TraceCategory::Consensus,
@@ -1270,12 +1287,6 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
prevLgr.id().kBytes,
link));
}
else
{
roundSpan_.emplace(
telemetry::SpanGuard::span(
telemetry::TraceCategory::Consensus, telemetry::seg::consensus, cs::op::round));
}
if (!*roundSpan_)
return;