Add response headers #365

dyladan · 2019-12-03T18:42:34Z

Preview | Diff

spec/21-http_response_header_format.md

tylerbenson · 2019-12-18T23:03:04Z

spec/21-http_response_header_format.md

+
+* `version`
+* `trace-id`
+* `parent-id`


I think using "parent" in a response is rather confusing. Is it referring to the span that initiated the request (and is likely receiving the response), or the span that is sending the response?

Depending on the answer, it should probably be renamed to something else.

It is actually neither. The motivational use case for this was the web browser initial page load. The page load happens before the instrumentation js is loaded, so the initial request is sent without a traceparent header. The first instrumented service can then create a "fake" span which it assigns to be it's parent and returns the id of the fake span to the client. The client then uses that id when it creates the span for the page load. So it is the id that is actually assigned to the parent span, making parent-id a decent name IMO.

I just wanted to chime in that we use response headers at SolarWinds (historically based on X-Trace headers used in Tracelytics, TraceView, and now AppOptics), and our response header includes a trace ID and event ID from the span that wrote the response in the header. So for us "parent" may not strictly be true (it's actually the child), but we can work with any name.

Basically, for us, the response header communicates a happened-before relationship between the end of the responding span and the end of the caller span, and also confirms the successful communication of the callee's response, as well as the caller's trace ID, between caller and callee.

@cce I think you're describing a substantially different use case.

"parent" makes sense to communicate back to the browser the ID it should use for its span (because for some reason it could not have created that ID before sending the request to the server)

@cce's case is to communicate back the server-side event ID

parent doesnt make sense. parent passed to a server is the parent of the server span. the span id generated is more often the child of the caller. returning something named parent in the response is confusing and not intuitive. focusing on what you know. ex the span id that serviced the request, is more sensible if this is included at all.

also bear in mind response header processing at all cant be mandatory. this isnt mentioned explicitly. if you do decide to do this it is making this not implementable without significant rewrites of code.

@adriancole @cce I think the issue may be that the name of the field does not fully convey it's intended semantic. Given the situation where service foo calls service bar, if bar returns a parent-id to foo, it is not saying "this is my span id", but rather "this is the span ID that you, my parent, should use."

The motivating use case is a situation like a browser where the initial request may not contain a span id, but the called service needs to use some id as its "parent," so it generates its own id but also generates an id that it uses as its parent. This parent id is then sent back up the wire to the calling service so that it can either use that id as its own span id, or it can at least link that id to its span.

Would changing the field name to something like suggested-parent-id, assumed-parent-id, requested-span-id or similar be more useful in conveying this semantic?

Here is a more visual explanation of the use case:

Let's rename this to proposed-parent-id (it's not like this string is sent over the wire, a longer name does not hurt).

I would also prefer that the spec clearly states that in case of the caller already providing well-formed traceparent header, the proposed-parent-id should be omitted.

https://github.com/w3c/trace-context/pull/365/files/41aa080c4d1a8c7132a8f8da2170dc861e6461ce..a412b5dd318f10baeab89933eae36e218a1958a8

spec/21-http_response_header_format.md

justinfoote

Overall, I love this PR. It matches what we discussed in Seattle, it has some great examples, it has good parity with the request header spec, and I love that you went back and updated the request header spec to add the word request.

I added some minor formatting comments, which you can take or leave.

Thanks for doing the legwork on this one, Dan!

spec/21-http_response_header_format.md

SergeyKanzhelev · 2020-02-11T22:50:14Z

spec/21-http_response_header_format.md

+The following `version-format` definition is used for version `00`.
+
+``` abnf
+version-format   = [trace-id] "-" [parent-id] "-" [trace-flags]


Making fields optional while keeping the same format as traceparent carry potential issues. Some may decide to use the same optionality for traceparent (even though it's not allowed by spec) and break interoperability. Mostly because the same code may be used to parse and generate both headers.

This said, it will be great to confirm that all the scenario listed below in this proposal:

Would actually significantly benefit from fields optionality.

Define a clear logic on how to decide which parts to return.

Would not need more properties than listed in traceresponse and will ACTUALLY need the arbitrary tracestate on response instead.

codefromthecrypt · 2020-02-12T01:39:51Z

as absence of parent implies root it is odd to create a routine to assume that we should always assume there is a caller. I have heard privately that another APM is able to do correlation without this mechanism. it would be good if folks can speak up rather than create an odd behavior case which assumes most requests are not only browser oriented but will be instrumented browser side

…

On Tue, Feb 11, 2020, 12:14 PM Daniel Dyla ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In spec/21-http_response_header_format.md <#365 (comment)>: > @@ -0,0 +1,190 @@ +# Trace Context HTTP Response Headers Format + +This section describes the binding of the distributed trace context to the `traceresponse` HTTP header. + +## Traceresponse Header + +The `traceresponse` HTTP response header field identifies a completed request in a tracing system. It has four fields: + +* `version` +* `trace-id` +* `parent-id` @adriancole <https://github.com/adriancole> @cce <https://github.com/cce> I think the issue may be that the name of the field does not fully convey it's intended semantic. Given the situation where service foo calls service bar, if bar returns a parent-id to foo, it is not saying "this is my span id", but rather "this is the span ID that you, my parent, should use." The motivating use case is a situation like a browser where the initial request may not contain a span id, but the called service needs to use some id as its "parent," so it generates its own id but also generates an id that it uses as its parent. This parent id is then sent back up the wire to the calling service so that it can either use that id as its own span id, or it can at least link that id to its span. Would changing the field name to something like suggested-parent-id, assumed-parent-id, requested-span-id or similar be more useful in conveying this semantic? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#365?email_source=notifications&email_token=AAAPVV3GEYYIMOUMK2XI3JDRCMBLTA5CNFSM4JU4XBV2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCVDM7GQ#discussion_r377873185>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAPVV763IWUGTXBGYTLDIDRCMBLTANCNFSM4JU4XBVQ> .

danielkhan

Great work @dyladan!

yurishkuro · 2020-03-25T18:13:16Z

spec/21-http_response_header_format.md

+``` abnf
+version-format   = [trace-id] "-" [proposed-parent-id] "-" [trace-flags]
+trace-id         = 32HEXDIGLC  ; 16 bytes array identifier. All zeroes forbidden
+proposed-parent-id        = 16HEXDIGLC  ; 8 bytes array identifier. All zeroes forbidden


nit: may want to fix whitespace/alignment

yurishkuro · 2020-03-25T18:15:26Z

spec/21-http_response_header_format.md

+
+#### proposed-parent-id
+
+This is the ID of the calling request as known by the callee (in some tracing systems, this is known as the `span-id`, where a `span` is the execution of a client request). It is represented as an 8-byte array, for example, `00f067aa0ba902b7`. All bytes as zero (`0000000000000000`) is considered an invalid value.


Suggested change

This is the ID of the calling request as known by the callee (in some tracing systems, this is known as the `span-id`, where a `span` is the execution of a client request). It is represented as an 8-byte array, for example, `00f067aa0ba902b7`. All bytes as zero (`0000000000000000`) is considered an invalid value.

This is the ID of the calling request proposed by the callee back to the caller in cases when the caller did not send the `traceparent` header (in some tracing systems, this ID is known as the `span-id`, where a `span` is the execution of a client request). It is represented as an 8-byte array, for example, `00f067aa0ba902b7`. All bytes as zero (`0000000000000000`) is considered an invalid value.

yurishkuro · 2020-03-25T18:31:21Z

Extra use case: when server creates a different trace (e.g. a cloud service), it may want to return its trace ID to allow the client to correlate with it.

yurishkuro · 2020-03-25T18:40:03Z

Merged as the current proposal. Open issues:

how to represent two distinct use cases: reporting server's trace/span ID vs. proposing span ID for the client
how to deal with optionality, especially given different use cases (maybe use dictionary)

codefromthecrypt · 2020-03-25T23:52:01Z

@yurishkuro let me get this right. you are literally going to expect people to speculatively generate parent IDs when creating a root span, which effectively means root spans won't have a null parent anymore? Then, add some middleware to push this back (more overhead), also speculatively as there's no way to know if the caller actually would use it. you are ok doing this without clearing this with with basically all OSS implementations except Otel which expect a root span to have no parent?

dyladan · 2020-03-26T00:05:16Z

@adriancole the working group decided on the call to merge this as a starting place today. In no way is it meant to be considered final and nothing is unchangeable. Also, note that the response header is an entirely optional part of the trace. If there is some material change you'd like to see feel free to join the call tomorrow or Friday (not sure what timezone you're in but the call is 2PM-5PM US East), file an issue, or open a PR.

codefromthecrypt · 2020-03-26T00:07:11Z

@dyladan I raised protest to this before you merged it and you merged it anyway without answering to that. what good would raising an issue be if you will just merge things anyway?

#365 (comment)

yurishkuro · 2020-03-26T00:35:48Z

expect people to speculatively generate parent IDs when creating a root span, which effectively means root spans won't have a null parent anymore

I expect this to be at minimum configurable in the SDK, and perhaps even triggered by something in the request. It's an odd use case with which I don't have any experience.

codefromthecrypt · 2020-03-26T00:39:37Z

In attempts to give a constructive way out, as this was merged IMHO by mistake.

Revert all text about proposed-parent-id, then re-introduce a PR with it, or better yet keep out until better studied and far more distinct stakeholders have a use case to pay for its complexity.

In normal OSS, you reduce change to what can be merged, and strip out stuff that isn't ready to be merged. That can work here, also even if retroactively.

codefromthecrypt · 2020-03-26T00:53:26Z

I also opened this, as I was surprised to find tracestate is a rare thing we are required to not allow unless accompanied by traceparent #401

dyladan added 2 commits November 20, 2019 15:53

Add initial work for response header

1a2953b

Move response header to its own file

f5a6406

yurishkuro reviewed Dec 3, 2019

View reviewed changes

spec/21-http_response_header_format.md Outdated Show resolved Hide resolved

yurishkuro reviewed Dec 3, 2019

View reviewed changes

spec/21-http_response_header_format.md Outdated Show resolved Hide resolved

yurishkuro reviewed Dec 3, 2019

View reviewed changes

spec/21-http_response_header_format.md Outdated Show resolved Hide resolved

fix wording

fd88d80

jmacd mentioned this pull request Dec 4, 2019

[http-plugin] Include trace ID in response headers for incoming requests open-telemetry/opentelemetry-specification#206

Closed

mtwo reviewed Dec 17, 2019

View reviewed changes

spec/21-http_response_header_format.md Outdated Show resolved Hide resolved

spec/21-http_response_header_format.md Outdated Show resolved Hide resolved

spec/21-http_response_header_format.md Show resolved Hide resolved

justinfoote reviewed Dec 17, 2019

View reviewed changes

spec/21-http_response_header_format.md Outdated Show resolved Hide resolved

dyladan mentioned this pull request Dec 17, 2019

Response header name #371

Closed

dyladan added 2 commits December 18, 2019 14:36

rename tracemeta to traceresponse

d102044

Make clear that fields are optional in response

d8067b7

tylerbenson reviewed Dec 18, 2019

View reviewed changes

Update spec/21-http_response_header_format.md

d7df516

plehegar mentioned this pull request Jan 24, 2020

Using Server-timing to carry TraceID w3c/server-timing#65

Closed

Fix wording for trace flags

98ccaeb

yurishkuro reviewed Jan 28, 2020

View reviewed changes

spec/21-http_response_header_format.md Show resolved Hide resolved

mtwo approved these changes Jan 28, 2020

View reviewed changes

Apply same wording change to request header

513f4cc

justinfoote approved these changes Jan 30, 2020

View reviewed changes

dyladan added 2 commits January 30, 2020 14:40

format use examples

40568d3

Merge remote-tracking branch 'origin/master' into add-response-headers

41aa080

SergeyKanzhelev reviewed Feb 11, 2020

View reviewed changes

danielkhan approved these changes Mar 25, 2020

View reviewed changes

danielkhan added discuss-next-meeting version-2 and removed version-2 labels Mar 25, 2020

dyladan added 2 commits March 25, 2020 13:51

change response parent-id to proposed-parent-id

1a9701a

trace id should be omitted from response if it is unchanged

a412b5d

yurishkuro reviewed Mar 25, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into add-response-headers

6212e44

danielkhan merged commit 4db5ee1 into w3c:master Mar 25, 2020

dyladan mentioned this pull request Mar 25, 2020

Should response header fields be optional? #398

Closed

dyladan deleted the add-response-headers branch March 26, 2020 00:05

dyladan mentioned this pull request Mar 26, 2020

[response header] Representing server span ID vs proposed parent ID #399

Closed

Legion2 mentioned this pull request Nov 4, 2021

Return traceId in header response. quarkusio/quarkus#11931

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add response headers #365

Add response headers #365

dyladan commented Dec 3, 2019 •

edited by pr-preview bot

Loading

tylerbenson Dec 18, 2019

dyladan Dec 19, 2019

cce Jan 6, 2020

cce Jan 6, 2020

yurishkuro Jan 28, 2020

codefromthecrypt Feb 5, 2020

dyladan Feb 11, 2020 •

edited

Loading

dyladan Feb 11, 2020

yurishkuro Mar 25, 2020

dyladan Mar 25, 2020

justinfoote left a comment

SergeyKanzhelev Feb 11, 2020

codefromthecrypt commented Feb 12, 2020 via email

danielkhan left a comment

yurishkuro Mar 25, 2020

yurishkuro Mar 25, 2020 •

edited

Loading

yurishkuro commented Mar 25, 2020

yurishkuro commented Mar 25, 2020

codefromthecrypt commented Mar 25, 2020 •

edited

Loading

dyladan commented Mar 26, 2020

codefromthecrypt commented Mar 26, 2020

yurishkuro commented Mar 26, 2020

codefromthecrypt commented Mar 26, 2020

codefromthecrypt commented Mar 26, 2020


		#### proposed-parent-id

		This is the ID of the calling request as known by the callee (in some tracing systems, this is known as the `span-id`, where a `span` is the execution of a client request). It is represented as an 8-byte array, for example, `00f067aa0ba902b7`. All bytes as zero (`0000000000000000`) is considered an invalid value.

	This is the ID of the calling request as known by the callee (in some tracing systems, this is known as the `span-id`, where a `span` is the execution of a client request). It is represented as an 8-byte array, for example, `00f067aa0ba902b7`. All bytes as zero (`0000000000000000`) is considered an invalid value.
	This is the ID of the calling request proposed by the callee back to the caller in cases when the caller did not send the `traceparent` header (in some tracing systems, this ID is known as the `span-id`, where a `span` is the execution of a client request). It is represented as an 8-byte array, for example, `00f067aa0ba902b7`. All bytes as zero (`0000000000000000`) is considered an invalid value.

Add response headers #365

Add response headers #365

Conversation

dyladan commented Dec 3, 2019 • edited by pr-preview bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dyladan Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinfoote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codefromthecrypt commented Feb 12, 2020 via email

danielkhan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro Mar 25, 2020 • edited Loading

Choose a reason for hiding this comment

yurishkuro commented Mar 25, 2020

yurishkuro commented Mar 25, 2020

codefromthecrypt commented Mar 25, 2020 • edited Loading

dyladan commented Mar 26, 2020

codefromthecrypt commented Mar 26, 2020

yurishkuro commented Mar 26, 2020

codefromthecrypt commented Mar 26, 2020

codefromthecrypt commented Mar 26, 2020

dyladan commented Dec 3, 2019 •

edited by pr-preview bot

Loading

dyladan Feb 11, 2020 •

edited

Loading

yurishkuro Mar 25, 2020 •

edited

Loading

codefromthecrypt commented Mar 25, 2020 •

edited

Loading