Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define behavior for response header context propagation #50

Closed
SergeyKanzhelev opened this issue Jan 17, 2018 · 14 comments
Closed

Define behavior for response header context propagation #50

SergeyKanzhelev opened this issue Jan 17, 2018 · 14 comments
Milestone

Comments

@SergeyKanzhelev
Copy link
Member

There are scenarios when services need to return correlation information in response. Scenarios include:

  • Sampling flag (+ sampling score) for delegated sampling
  • Tenant ID/identity of the service so caller knows where to query telemetry from

Spec need to define which headers SDK may expect and service should use for http response. Also behavior for service mash and proxies needs to be defined so headers would not be lost.

@codefromthecrypt
Copy link

codefromthecrypt commented Jan 17, 2018 via email

@SergeyKanzhelev
Copy link
Member Author

Yes, that was the main point. We need information that is required to build a trace to be returned back in response in certain scenarios. First scenario is for sampled flag that is part of Trace-Context header and second is about solution specific tenant id that is part of Trace-Context-Ex.

And we need to make sure those headers will be preserved and passed further in response for proxies and balancers. But we cannot demand it from all services.

Also - does it ever make sense to return trace-id in response headers? Or we can limit spec to only allow Trace-Context-Ex header to be returned.

@codefromthecrypt
Copy link

codefromthecrypt commented Jan 18, 2018 via email

@SergeyKanzhelev
Copy link
Member Author

Ok. So scenario for trace-id I understood from the linked issue is: "let's return trace-id if we started a new one". This may be valid when service do not trust caller to define a trace, but trust enough to expose the newly generated one. I think this may be preferable mode of operation for some cloud services. So it would be great to allow for this implementation in a standard.

So I'd say let's just state that both headers can appear in response. And implementors can do what they need. If you do not know what to do with them and have 1:1 mapping between incoming and outgoing span - pass context along.

@SergeyKanzhelev
Copy link
Member Author

It may be good enough for a spec. We can mention specific use cases as a best practices.

@wu-sheng
Copy link
Contributor

wu-sheng commented Jan 18, 2018

response header context propagation

@SergeyKanzhelev @adriancole This is an ongoing design in SkyWalking about this concept. But not relate to tenant id or new generated trace-id.

In our case, it supposes to be used like this:

  • Pass back the server side service name(operation name of EntrySpan). e.g. in Spring HTTP mapping, server side URL can be defined like prod/{userId}/{orderId}, at the same time, client side use prod/123/o345 as URL. You can see there are different in both side. In SkyWalking analysis module, it asks for no blocking/waiting other trace segment in order to improve performance. But as different URLs used, the service dependency is not right.

@yurishkuro
Copy link
Member

What would be the semantics with server returning a trace ID if the client also sent a trace ID in the request? Will the spec require that ID to be the same? What about span ID?

One use case someone raised recently was about using the returned trace+span IDs to validate that the response is actually to the right request. To quote the source, "I've experienced Java bugs before where we were sending responses on the wrong file descriptors which causes all sorts of trouble." Arguably this could be delegated to be the RPC framework responsibility, at the expense of RPC framework injecting yet another unique ID header.

@SergeyKanzhelev
Copy link
Member Author

@wu-sheng are you thinking of operation name in response as a Trace-Context-Ex property or Correlation-Context property? (With the guidance that trace context is used for tracing essential properties and very limited in size). I can see it as a scenario to allow Correlation-Context header in response.

@yurishkuro scenario I suggested is when service do not trust client to send random enough trace-id or has it's reasons to restart the trace. @bogdandrutu was talking a lot about Google services doing it. In this scenario returning different trace-id will make it possible to link items. Scenario of RPC calls request/response matching may be also valid. However I do not think the spec should require the match.

@yurishkuro
Copy link
Member

However I do not think the spec should require the match.

I think that's fine, we just need to be explicit about it, not leave undefined behavior/expectations.

@SergeyKanzhelev
Copy link
Member Author

@yurishkuro @adriancole I;'d appreciate review of PR #51

@bogdandrutu
Copy link
Contributor

I know some other things that we may want to have in the response:

  • server latency (when you cross trusting boundaries) you may want to share this with your client, e.g. one cloud service can return this.

@SergeyKanzhelev yes we do have this problem about trusting the incoming trace-id.

@bripkens
Copy link

bripkens commented Feb 1, 2018

server latency (when you cross trusting boundaries) you may want to share this with your client, e.g. one cloud service can return this.

This could be achieved via Server-Timing: https://w3c.github.io/server-timing/

Actually, Server-Timing could be (ab-)used to implement some of the use cases mentioned here.

@SergeyKanzhelev
Copy link
Member Author

Also mention populating of Access-Control-Expose-Headers header

@SergeyKanzhelev SergeyKanzhelev added this to the experiment milestone May 2, 2018
@AloisReitbauer
Copy link
Contributor

This relates to whether we can reuse server timing #69

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

No branches or pull requests

7 participants