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

Implement in-process context propagation #43

Closed
jcchavezs opened this issue Nov 16, 2017 · 8 comments · Fixed by #47
Closed

Implement in-process context propagation #43

jcchavezs opened this issue Nov 16, 2017 · 8 comments · Fixed by #47

Comments

@jcchavezs
Copy link
Collaborator

jcchavezs commented Nov 16, 2017

Background

OpenTracing API aims to provide interfacing for in-process context propagation. After several discussions in the Java repo, finally OT has a clear design for it.

Proposal

There is a PR in Python aiming to fix this. We might ship something very similar.

Questions to address

  1. What will be the relationship between the new active span with respect to the current one?

Ping @tedsuo @felixfbecker @beberlei @lvht

@taoso
Copy link

taoso commented Nov 16, 2017

@jcchavezs

OpenTracing 1.x leaves the question of in-process context propagation entirely in the hands of the applications themselves

I propose not to introduce the in-process context propagation feature until the release v2.0.0.

@beberlei
Copy link
Contributor

without in process propagation opentracing is useless. there is no easy way to access the current span deep in your code and especially not within third party libraries.

@taoso
Copy link

taoso commented Nov 19, 2017

@beberlei

there is no easy way to access

Yes.

without in process propagation opentracing is useless

No, absolutely! Tracing the current span with a stack like array is not so hard as you imagine.

I agree that in process propagation opentracing is a great convenient. However, as @yurishkuro said in the issue opentracing/specification#23,

OpenTracing 1.x leaves the question of in-process context propagation entirely in the hands of the applications themselves, which substantially reduces the usefulness of the standard since different frameworks instrumented with OT do not have a way to exchange context.

It is no doubt that the OT spec will support the in process propagation opentracing. However, the the issue opentracing/specification#23 is added to the OpenTracing 2.0 milestone.

So as a PHP-version of the OT-spec, we should release our v1.0 as soon as possible. And the feature of in process propagation opentracing should be added in our v2.0 as well.

@jcchavezs
Copy link
Collaborator Author

@lvht I strongly believe not including in-process context propagation (IPCP) is a mistake. IPCP is what keep us away to the instrumentation of common libraries in PHP (for example, http client or db connections in PHP) because as @beberlei said there is no way to access to the current context. I would reword @beberlei's words as OT without IPCP is not something that will reach production support because in that case we might need to propagate the context down in the stack which is unnatural in PHP (as opposed to Golang) and having to spread the tracing context in business logic is something people won't do and will make the adoption very inconvenient.

In the other hand, since there is a lot of effort around supporting IPCP in all languages (except golang) we will support that very soon, meaning that all the effort spent on instrumenting use cases passing the context (which is an overhead in terms of development) will be a waste of time.

@taoso
Copy link

taoso commented Dec 3, 2017

@jcchavezs I propose to froze the current API by making a v1.0.0 release, and finish the current stable milestone. All other features discussing can be added in the future v1.x or v2.x.

@taoso
Copy link

taoso commented Dec 6, 2017

ping @jcchavezs The 2018 is coming

@taoso
Copy link

taoso commented Dec 27, 2017

ping @jcchavezs

1 similar comment
@taoso
Copy link

taoso commented Jan 4, 2018

ping @jcchavezs

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 a pull request may close this issue.

3 participants