jaegertracing / jaeger

CNCF Jaeger, a Distributed Tracing Platform
https://www.jaegertracing.io/
Apache License 2.0
20.15k stars 2.4k forks source link

Jaeger client for PHP #211

Closed yurishkuro closed 3 years ago

yurishkuro commented 7 years ago

OpenTracing API for PHP is available: https://github.com/opentracing/opentracing-php

jcchavezs commented 6 years ago

Finally we have https://github.com/opentracing/opentracing-php, shall we update the issue @yurishkuro ?

black-adder commented 6 years ago

Just FYI, we do have someone currently building out the php client: https://github.com/jukylin/jaeger-php

otisg commented 6 years ago

@jcchavezs are you referring to https://github.com/jcchavezs/jaeger-client-php ? From what I can tell, https://github.com/jukylin/jaeger-php is further along. Maybe joining forces with @jukylin would be better?

jukylin commented 6 years ago

It's good idea .jukylin/jaeger-php depend on opentracing/opentracing-php, and we can make it more better together.

jcchavezs commented 6 years ago

I believe this one is looking great: https://github.com/jonahgeorge/jaeger-client-php I am thinking on joining @jonahgeorge.

taoso commented 6 years ago

The opentracing/opentracing-php is far from stable. Even it has a 1.0.0-beta release, its latest master branch breaks the API.

So I suppose to only depend on it when it hits a 1.0.0 release.

felixfbecker commented 6 years ago

The opentracing/opentracing-php is far from stable.

And so is jaeger-php. There is no reason to not start developing the Jaeger implementation. It's easy to change the API.

taoso commented 6 years ago

@felixfbecker It's easy to change the API until you use them in production environment.

felixfbecker commented 6 years ago

If you use a version clearly documented as unstable in production that's your own risk and no reason to artificially slow down development progress.

yurishkuro commented 6 years ago

In https://github.com/jaegertracing/jaeger/issues/366#issuecomment-348703406 @lvht said:

would you like to accept the lvht/jaeger-php as the official PHP client of Jaeger?

I would like to hear from other interested parties. I myself don't have a strong opinion since we do not write applications in PHP at Uber.

tylerchr commented 6 years ago

We have quite a bit of duplicated work going on here (or parallel efforts, depending on your perspective):

Each of these are in various states of feature completeness, stability, opentracing-php compatibility, PHP5/7 compatibility, production-readiness, and maintenance.

Rather than balance many separate projects with one owner each, I propose we make an effort to merge these all into one repo with many owners. This will produce a much higher-quality client than if we each go it alone. @jonahgeorge and I have previously discussed merging our efforts, so maybe that's a place to start. Barring any objections, I'd be happy to own facilitating that.

jukylin commented 6 years ago

It's interesting

yurishkuro commented 6 years ago

100% in favor of merging the efforts into an official repository under https://github.com/jaegertracing.

Might be useful to have a feature comparison among the existing implementations, pros/cons. The parent ticket #366 lists the common requirements for an official client lib.

allflame commented 6 years ago

@tylerchr first of all, I'm personally in favor of combining efforts into a single library. However, when I was looking over for existing implementations I failed to find one that was well-enough built and used a stack-based approach to automatically define parent-child span relationship.

I'm not sure we can advise to use our library at this point since it lacks documentation and tests, but we are proud of API stability that we were able to achieve.

felixfbecker commented 6 years ago

Could you explain what you mean by stack-based approach? How would that work with concurrent programming or an event loop?

yurishkuro commented 6 years ago

I assume "stack-based" refers to some form of active span management, which is important but should be solved at the OT API level, just like it was in Java/Python. I think that concern is very different from actually having an official PHP client for Jaeger, even if it doesn't have active span management features yet (because official PHP OT API doesn't have them).

allflame commented 6 years ago

@felixfbecker you start a span - an active context is put on stack, you finish span - you pop the context from the stack. This was (our own) solution coming from requirement not to pass context from layer to layer in the application itself. This is discussed and "rationalized" in some go applications, but it's really different language where this is a general approach. About concurrency and event-loops: of course, your tracer should be thread-safe in order to work properly there. This is also mentioned in Opentracing documentation. Since you can still inject context and basically clone your tracer in another thread, I don't think this is a concern until async/await support. As for event loop - I specifically added a possibility to manually specify parent context.

@yurishkuro You are completely right. This was never thought to be an "official" implementation. It does follow the formal requirements and have all necessary (at least for my use-case) features

tylerchr commented 6 years ago

The stack abstraction doesn't work for concurrent or event-loop-based languages, but PHP isn't concurrent or event-based. As such, a stack does model the in-process lifecycle of a trace very neatly for PHP, in what I believe is a closed-form way. A "push" starts a new span (which is a child of the span on the top of the stack if one exists), and a "pop" finishes the top span on the stack. It also has the nice advantage of providing a way to identify all the unfinished spans and close them, in unusual cases like when the script ends without completing execution (usually because someone called die() or exit(); in such cases it's nice to not lose your whole trace because the root span never got finished).

We did the same thing at Qualtrics (see this gist for our basic approach), but our stack implementation is outside our jaeger-client-php implementation and just works on the opentracing-php API. This seemed ideal because it offered a nice separation of concerns, without forcing my stack implementation on all users of my jaeger-client-php package. To @yurishkuro's point, should some kind of "active span management" functionality be standardized for PHP, it'll be relatively easy for us to replace.

jukylin commented 6 years ago

I release to v2.0.0-beta https://github.com/jukylin/jaeger-php

sgnrslv commented 6 years ago

Hi there.

We've implemented Jaeger client as PHP extension.

Probably, there is a lack of compatibility with all OpenTracing API concepts, but the extension covers all our needs and proved itself in a production battle ;)

So feel free to give it a try.

sagikazarmark commented 6 years ago

An extension sounds like a great idea, however I'm afraid that in many cases it's simply not an option. It would be awesome though if there would be a userland implementation which could be installed as a fallback when the extension is not available.

Anyway: I haven't worked on a Jaeger PHP client (yet), but I'm willing to join the coordination effort if it brings us any closer to the solution. (Maybe it even helps that I'm not biased 😉 )

I briefly looked into the libraries listed by @tylerchr , they all seem to follow similar concepts. There are differences in quality and implementation of course.

I agree with @yurishkuro that the first step could be a feature comparison. I would also take a look at the concrete solutions as well.

I know some of you expressed support for one or another libraries, but personally I think the best thing to do at this point is to agree on some key concepts based on the above comparisons, and start building a new library under the official jaegertracing org, even if that means copying over a bunch of source code.

That way everyone can equally contribute to the effort, the current libraries stay intact and even when copying source code, we can review the pieces again one by one.

isaachier commented 6 years ago

I'd highly recommend the extension route if possible. We already have a C++ client and I've been working on a pure C client as well.

isaachier commented 6 years ago

@sagikazarmark what you do mean "would not be an option." A huge amount of PHP is implemented in terms of C extensions.

sagikazarmark commented 6 years ago

In many environments custom built PHP extensions are not allowed or not possible for whatever reasons.

isaachier commented 6 years ago

We could package it via pear or wtvr

sagikazarmark commented 6 years ago

I guess it's not a coincidence that many extensions have a userland implementation as well (protobuf, twig to name a few). I'm not arguing that an extension would be better for a number of reasons. But it doesn't suit everyone.

isaachier commented 6 years ago

I understand. We will see what the community produces and evaluate any clients for a pure PHP version.

tylerchr commented 6 years ago

I started working on a feature comparison at the end of last year and never published it. It hasn't been updated since December, but only jonahgeorge/jaeger-client-php and code-tool/jaeger-client-php seem to have had any serious development since then. Perhaps @jonahgeorge and @allflame can update it?

https://docs.google.com/spreadsheets/d/19OTyj63I3YfLa8h4KGsAvljvA_AZDkhSeXYkFlPbnjg/edit#gid=0

@sagikazarmark's suggestion that we align on a set of core principles and then collaborate on a new implementation seems wise. I do suspect that a lot of code from (or derived from) the current implementations will wind up there, but that's just fine if we maintain a consistent bar of quality and polish.

sagikazarmark commented 6 years ago

This is really nice!

allflame commented 6 years ago

@tylerchr Great idea! So as for our implemnentation:

  1. It does support PHP 5.6. Specifically, there is a separate branch 1.0.x with all features backported there - mainly as the consequence of semantic versioning.
  2. Not sure what does "Configuration" stands for. We tend to configure everything with DI containers like this. There is no standalone class like Factory::fromConfig if that's what you're talking about (we do not like static calls)
  3. I'm not sure why you included HTTP/UDP support here since this is a part of Thrift client library itself. We did write our own UDP one to suppress some error message and remove a lot of (from our point) complicated and unnecessary code. Message: I don't think this should be included in the table, maybe only as "Additional features"
  4. We do have all the samplers that are listed under the opentracing.io "Sampling" section including
    • Const
    • Probabilistic
    • RateLimiting
    • Adaptive

Latter 2 require apc extension (I think there is no way around to use something even more lightweight if you running it like fpm/cgi/mod and not standalone like react/php-pm/swoole). This will result in a side effect of a minimum Y*X instead of X traces if you have Y installation of application and set adaptive sampler rate to X. Of course we do not have the Remote sampler since I think it was not opensourced by Uber yet. We will implement Baggage in the next future just to add to the supported features, it's just we don't use it.

P.S. We can also build a bridge package to comply with opentracing-php, if someone feels like that would be useful.

dragoscirjan commented 5 years ago

Hello! How's the status on this one? Did anyone complete anything?

asp24 commented 5 years ago

Hello! How's the status on this one? Did anyone complete anything?

All packages in code-tool org which contains "jaeger" in the name -- stable and works for a long time in production. So from my point of view, it's completed

dragoscirjan commented 5 years ago

@asp24 I'm not sure I understand your answer. If there is an official PHP jaeger client, I'm not sure where I could find it (if not on this project page). Furthermore, I see both major developers from the discussion above @jukylin and @jonahgeorge continue to develop on their own.

So my question still stands.

yurishkuro commented 5 years ago

@dragoscirjan we have had a similar situation with the C# client, where a couple of different repositories existed implementing the client, but their authors agreed to join the efforts, which resulted in an official client under the jaegertracing org maintained by the original authors. Something similar needs to happen here; we as core maintainers cannot make the call on which project is a better fit to become official client lib (since none of the core maintainers work with PHP). The first step towards that would be a feature comparison matrix.

jonahgeorge commented 5 years ago

If we can get @jukylin's repo accepted into the jaegertracing org, I'm happy to sunset mine (and join forces)

sergeyklay commented 5 years ago

I'm agree with @jonahgeorge

jpkrohling commented 5 years ago

I added the topic of the PHP client to the next Jaeger Project Bi-Weekly meeting: http://bit.do/jaeger-call

Interested parties: please join the meeting, ideally with an agreement about which repo to accept. I'd suggest following @yurishkuro's advice in building a feature comparison matrix, perhaps including the 9 guideline items listed under https://github.com/jaegertracing/jaeger/issues/366 . I'm afraid none of the current maintainers have enough PHP skills to judge and pick one implementation over the other ones.

cc @yuklin @jcchavezs @jonahgeorge @tylerch @lvht @allflame @korchasa

LPodolski commented 5 years ago

jukylin's repo still does not support beta 5 of open tracing, https://github.com/jukylin/jaeger-php/issues/21, jonahgeorge repo is better in that aspect

CpuID commented 5 years ago

Was there a decision made on which implementation is likely to go official...?

jpkrohling commented 5 years ago

IIRC, nobody showed up at the meeting. I'm closing this issue, as there seems to be demand from users but no volunteers to organize the merge.

jpkrohling commented 5 years ago

Sorry, it was wrong to close this. We have similar issues for other languages tracked as part of #366 and the issues are kept open until a client is accepted.

zzarbi commented 5 years ago

Just poking around but it seems that @jukylin 's version make the most sense, we can always bring back support for beta 5 into it. Furthermore he's still really active in his repo since the last 3 years. With that said thank you everybody for working on a solution. @jpkrohling what would you recommend we do in order to move forward on topic?

yurishkuro commented 5 years ago

Can we get a sense of how many people / companies are using https://github.com/jukylin/jaeger-php ?

zzarbi commented 5 years ago

@yurishkuro How do you want to collect this information?

yurishkuro commented 5 years ago

Reply here, ideally stating name of your company, type of usage (dev, prod, etc.), # of apps/services where lib is deployed, traffic volume if possible.

zzarbi commented 5 years ago

Company Name: Thrive Market Usage: Dev, but ideally prod when we have stable version Number of Service in PHP: +3 Volume: Our main service in staging is about 500request/minute Current status: Development

jukylin commented 5 years ago

I know some companies using https://github.com/jukylin/jaeger-php. Maybe I can ask them for help.

pavlelee commented 5 years ago

Company Name: uxin Project Name: After sale、Part of uxin finance Usage: prod Number of Project in PHP: +10 Volume: 40~100rps/service

jukylin commented 5 years ago

Company Name: ETC Project Name: all Usage: prod Number of Project in PHP: +100 Volume: 1k/s

yurishkuro commented 5 years ago

@jukylin just to confirm, are you interested in moving https://github.com/jukylin/jaeger-php to https://github.com/jaegertracing ?