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

Remove proposed-parent-id from response header #406

Closed
dyladan opened this issue Mar 27, 2020 · 7 comments · Fixed by #443
Closed

Remove proposed-parent-id from response header #406

dyladan opened this issue Mar 27, 2020 · 7 comments · Fixed by #443
Assignees

Comments

@dyladan
Copy link
Member

dyladan commented Mar 27, 2020

After feedback from @adriancole and conversations during the march online workshop, it was decided to remove the proposed parent id for now.

@yurishkuro
Copy link
Member

Was there an alternative mechanism proposed to solve the original "web browser" issue? Please document it here, for posterity.

@dyladan
Copy link
Member Author

dyladan commented Mar 27, 2020

There is not. The issue is that there currently wouldn't even be a mechanism in browsers to use this feature, so until we get sign-on from browser vendors it won't be useable. You can't access arbitrary headers from the initial page load.

@yurishkuro
Copy link
Member

I was under the impression that this was an already existing use case, e.g. traceparent usage described here: https://github.com/census-instrumentation/opencensus-web

@dyladan
Copy link
Member Author

dyladan commented Mar 28, 2020

There are a handful of methods including cookies, server-timing, and injecting meta tags in web pages for communicating proposed parent ids back to the caller, but as far as I can tell, no API exists to read arbitrary response headers from the initial document load request.

@dyladan
Copy link
Member Author

dyladan commented Jul 28, 2020

This has been discussed in several of the bi-weekly meetings now. I'm updating this issue to the current state of the discussion.

We are currently hoping to keep this use-case in, but it depends on adding new browser APIs. We would like to solidify the use-case and try to see if browsers would implement this. If we cannot get browser adoption, there is no way to access the proposed parent from within the browser and the field should be removed.

@codefromthecrypt
Copy link

codefromthecrypt commented Oct 9, 2020

TL;DR; I think this spec by adding features like proposed parent ID jumped too far without enough eyes on the problem. This is very much implementation not api. cc @cwensel and @jcchavezs

we can't say there's no solution as there have been very few independent minds on the problem, or folks offering solutions. It is moreover hard to help anyone solve this as discussions continue with no visible code to help people with. I don't understand how something can be added to a spec based on something no one can see. you just have to take someone's word on it? This should have an example code so people can help you.

Anyway, there's a massive difference between returning the context that serviced the request (its ids) and generating a "proposed parent ID" for a potential instrumented client which may never exist. The latter is something that shouldn't be in the spec at all. There are other ways, even if vendors doing browser based tracing today are not sharing their processes.

Here's one for zipkin:

In zipkin, only traceId and spanId are immutable fields. This means parentId is an updatable field: you can patch it the same as adding a late tag. It is just a field.

If you had the server's context (it thinks is root) sent back to you as b3: 0412f83c4ae74844-0412f83c4ae74844-1

This tells the browser, that the server sampled it (trailing -1 means sampled) so it is worthwhile rewriting your pending spans. vs doing nothing.

Say you have a cache of data. Knowing it should be reported, you rewrite the trace IDs to 0412f83c4ae74844, and then...

you patch the server's span adding the parentId of the outgoing one you feel should be the parent's spanId.

By patching the server span, (parentId=null -> parentId=clientSpanInBrowser.spanId), it is demoted from root to a child span.

Ex. given the server context "b3: 0412f83c4ae74844-0412f83c4ae74844-1" and a browser client ID "e8dd8907018965be", in my json I posted back I would include a patch for the server span:

{
  "traceId": "0412f83c4ae74844",
  "id": "0412f83c4ae74844",
  "parentId": "e8dd8907018965be"
}

Aggregation and also UI already merges spans by their IDs. The system would never know the difference.

@dyladan
Copy link
Member Author

dyladan commented Dec 1, 2020

At the fall 2020 workshop, we were unable to present a use-case for the proposed parent ID that could not be solved in another manner.

It was decided to remove it in favor of the span id of the child. The child span id assists in the disambiguation of multiple calls to the same service provider within the same trace for the support use-case. I am working on an updated version without the proposed parent ID and expect to have a PR opened in the next day or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants