Skip to content

proposal: Introduce Trace context Propogation - #4

Open
VaishnaviHire wants to merge 1 commit into
praxis-proxy:mainfrom
VaishnaviHire:trace_context_proposal
Open

proposal: Introduce Trace context Propogation#4
VaishnaviHire wants to merge 1 commit into
praxis-proxy:mainfrom
VaishnaviHire:trace_context_proposal

Conversation

@VaishnaviHire

Copy link
Copy Markdown

What does this PR do?

Discussion link

Checklist

  • Discussion link is in proposal frontmatter
  • Issue link is in proposal frontmatter
  • Authors and stakeholders listed
  • Repos list includes all affected repositories
  • Graduation criteria are specific and verifiable
  • First PR contains only What? and Why? (no How?)

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

5 findings (1 critical, 2 large, 2 medium). The proposal is well-motivated and the What/Why sections are clearly scoped. Main blockers: missing required discussion link, incomplete W3C Trace Context coverage (tracestate omitted), and the issue field pointing to a PR rather than a tracking issue.

Comment thread proposals/00654_trace-context-propagation.md Outdated
Comment thread proposals/00654_trace-context-propagation.md Outdated

- `x-request-id` — request correlation ID used primarily for logs and
support/debugging.
- `traceparent` — W3C Trace Context header used to continue a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Large] The W3C Trace Context specification defines two headers that conforming implementations must propagate together: traceparent and tracestate. Per Section 3.3 of W3C Trace Context Level 1: "If a tracing system receives a W3C Trace Context with a traceparent header, it MUST propagate the tracestate header if received."

The proposal only lists traceparent. Add tracestate to the propagated headers list, or explicitly list tracestate as a non-goal with rationale for why partial W3C conformance is acceptable at this stage.

absent or malformed.
- Propagate `x-request-id` and `traceparent` to the forwarded upstream
request when enabled.
- Propagate `x-request-id` and `traceparent` to subrequests

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Medium] The W3C spec requires each participating system to update the parent-id field in traceparent to produce a hierarchical trace graph. If subrequests copy traceparent verbatim, the trace topology will be flat -- all legs appear as siblings rather than children of the originating request.

Add a goal or note clarifying the intended span relationship: should subrequests generate a fresh parent-id (proper parent-child hierarchy in Jaeger/Tempo) or propagate traceparent unchanged (flat correlation only)? This distinction fundamentally affects trace topology and should be stated in the What, even if the mechanism is deferred to the How.

Comment thread proposals/00654_trace-context-propagation.md Outdated
Signed-off-By: Vaishnavi Hire <vhire@redhat.com>

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Review (Follow-up)

Summary: The discussion and issue tracking links are now correct. However, two W3C Trace Context conformance issues from the previous review remain unaddressed: tracestate header propagation is still missing, and the proposal does not clarify whether parent-id will be updated to create hierarchical traces.

Severity Count
Large 1
Medium 1

- `x-request-id` — request correlation ID used primarily for logs and
support/debugging.
- `traceparent` — W3C Trace Context header used to continue a
distributed trace across services.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Large] The W3C Trace Context specification (Level 1, Section 3.3) defines two headers that conforming implementations must propagate together: traceparent and tracestate. The spec states: "If a tracing system receives a W3C Trace Context with a traceparent header, it MUST propagate the tracestate header if received."

The proposal lists only traceparent (line 52). The tracestate header carries vendor-specific trace context that must not be dropped when forwarding or creating subrequests.

Either:

  1. Add tracestate to the propagated headers list (lines 47-52) and Goals (lines 65-71), or
  2. Explicitly list tracestate as a Non-Goal with rationale for why partial W3C conformance is acceptable at this stage (e.g., "v1 propagates traceparent only; tracestate deferred to post-v1 after OTel integration is validated").

- Propagate `x-request-id` and `traceparent` to the forwarded upstream
request when enabled.
- Propagate `x-request-id` and `traceparent` to subrequests
transparently through the default subrequest path when enabled.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Medium] The W3C Trace Context spec requires each participating system to update the parent-id field in traceparent when creating child spans to produce a hierarchical trace graph. If subrequests copy traceparent verbatim (without updating parent-id), all trace legs will appear as siblings rather than children of the originating request in tools like Jaeger or Tempo.

Line 70-71 states "Propagate x-request-id and traceparent to subrequests" but does not clarify whether propagation is verbatim or involves parent-id generation.

This distinction fundamentally affects trace topology and should be stated in the What/Why phase. Add a note or Goal clarifying the intended span relationship:

  • "Subrequests generate a fresh parent-id to create proper parent-child hierarchy in distributed traces", or
  • "v1 propagates traceparent unchanged for flat correlation; hierarchical span relationships deferred to future OTel span integration."

This is not an implementation detail — it's a core design choice that affects whether the feature delivers hierarchical tracing or just correlation.

@leseb leseb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good thing for observability.

@cdoern cdoern left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm. thanks for making this!

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.

6 participants