opentracing / opentracing-php

OpenTracing API for PHP
Apache License 2.0
505 stars 56 forks source link

In process propagation #2

Closed jcchavezs closed 7 years ago

jcchavezs commented 7 years ago

This is related to the in_process_propagation from: opentracing/opentracing-python#52

The specification context is "In-process propagation" discussed in this issue opentracing/specification#23, already for Java and in PR for Python.

Ping @jcchavezs @felixfbecker @tedsuo @beberlei

tPl0ch commented 7 years ago

Hi, as a possible implementer I am asking myself if it makes sense waiting for this PR to be merged. When do you plan to add this?

jcchavezs commented 7 years ago

@tPl0ch nice to have you around. So basically there is a lot of buzz around this concept of in process propagation mostly in Java (https://github.com/opentracing/opentracing-java/pull/166) and even the python implementation hasn't been merged yet. Although most of the concerns discussed in the Java PR does not affect us (as we don't have threads) it feels like there is not a full agreement on the main concept so we kind of decided to hold on this PR until there is a solid agreement on the final spec and then we implement what is needed.

It is up to you to wait on this one but I encourage you to start doing it right now and give us some feedback on your experience using this library, I'd tell you that if we merge this PR we won't include breaking changes so even if we merge it in the mean time it won't end up on too much rework.

Finally I'd like to mention that this is mostly a RC yet, but we plan to release it very soon (with small tweaks but nothing substantial) so feel confident to start using it. It would be also nice if you can come and say hi to our gitter room: https://gitter.im/opentracing/opentracing-php

tPl0ch commented 7 years ago

@jcchavezs The reason why I am asking is that when I was browsing the repository I realized that some of the features developed in this PR have leaked into the master branch. See https://github.com/opentracing/opentracing-php/blob/master/src/OpenTracing/SpanOptions.php#L6 - ChildOf is not defined.

Generally I am also missing some statements about the stability of this library. Is this lib going to follow a semantic version approach?

EDIT:

Additionally, this PR would break the Tracer API, see https://github.com/opentracing/opentracing-php/pull/2/files#diff-bd85a3792bd64b152939e0b1b78086e2R32 .

EDIT 2:

Although most of the concerns discussed in the Java PR does not affect us (as we don't have threads)

See http://php.net/manual/en/intro.pthreads.php. It is completely possible to do threaded programming in PHP (the question if you should is another one, though).

jcchavezs commented 7 years ago

@tPl0ch I am sorry I did not give you more context on this PR. Basically this branch is outdated and I think I will close this for now until the in process propagation is fully defined so we avoid confusions. Regarding the missing features in master, let me clarify it: this branch was created a month ago when the API was slightly different than now (see https://github.com/jcchavezs/opentracing-php/pull/22) so some concepts were changed since then (one of the more visible ones was the fact that now we don't have specific classes per reference type but we have one Reference class). In addition, when this branch was created we expected this concept to be already settled and we accounted for the Python implementation to be merged very soon (which was not the case). In summary, whenever this PR is ready to be taken into account, it will be open for people to discuss but it should not be subject of discussion for now (this is on me).

Regarding the threads in PHP I knew about this pthreads library in V2 but I have the impression (correct me if I am wrong) that it is not wide used and don't know much about production systems using it but if there is a real interest in it we could include it later.

Finally, we are discussing now the versioning for this library. To be clear, what we have now is mostly what we might have as a first release and soon we will tag this code as the initial version (there are high probabilities to call it 1.0-beta1 or something like that) and yes, we will of course use semver

If you are about to (or up to) work on an implementation of opentracing in PHP come and talk with us in the gitter channel and we will more than happy to listen to you and solve any doubt you might have.

jcchavezs commented 7 years ago

Closed for now.