opentelemetry-php icon indicating copy to clipboard operation
opentelemetry-php copied to clipboard

Create OpenTracing Shim

Open tidal opened this issue 2 years ago • 5 comments

Specification Reference: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/opentracing.md OpenTracing PHP repository: https://github.com/opentracing/opentracing-php

tidal avatar Jul 02 '22 21:07 tidal

I'm guessing this one doesn't need to be thought out from scratch too much, and just following along with another language's implementation (or 2) with the spec at hand will get 90% of the way there.

Like for JS it looks like they managed this in roughly 400 lines of TS code - https://github.com/open-telemetry/opentelemetry-js/tree/f59c5b268bd60778d7a0d185a6044688f9e3dd51/packages/opentelemetry-shim-opentracing

If no one's able to pick this up after the next week or so, I might be able to and take a crack at it

Grunet avatar Jul 06 '22 02:07 Grunet

@Grunet - are you still planning on working on this?

bobstrecansky avatar Aug 10 '22 12:08 bobstrecansky

Am a little personally underwater atm (ideally will not be in ~2-3 weeks) but once not was planning to take a stab at #723 first.

But if this is a higher priority I can definitely try this first (and announce when I'm starting on it).

Otherwise totally cool if someone else wants to take a shot. I did not look into it too deeply before.

Grunet avatar Aug 11 '22 01:08 Grunet

Looking at it more closely now, I have 2 other observations

  1. JS and Python keep all of the Shim classes in a single file, but Java separates them out into their own files with the usual 1 per class deal. Since PHP/this repo has the same style, Java is probably the example to follow most closely.
  1. The other repos all seem to publish a separate package for this shim, which the spec itself seems to kinda mandate when it says "This functionality MUST be defined in its own OpenTracing Shim Layer, not in the OpenTracing nor the OpenTelemetry API or SDK". So presumably the same needs to be done here, meaning the "gitsplit" setup needs to be extended I believe (initially added in this PR).

I'm starting at a new gig next week so won't have much time right after that probably, but I do have a day off before then, so I will see how far I can get with 1) and then drop any notes on partial/progress here

Grunet avatar Sep 12 '22 01:09 Grunet

So there are a total of 27 methods across 5 interfaces that you need to fill out to make any OpenTracing-PHP implementation (including an OTEL-backed one as is needed here)

Scaffolding the initial files and taking a stab at the first 7 methods for OpenTracing's "Tracer" interface ended up taking me roughly around 7 hours (a large chunk of which was trying to understand-then-translate Java and JS's behaviors for the methods I didn't conceptually understand as well up front. I'm looking at you propagation lol). I also totally skipped some things for that interface implementation (like translating span creation options between OpenTracing and OTEL) just to try to get something in place for all of the methods.

You can see where I got to here - https://github.com/Grunet/opentelemetry-php/pull/6/files

I sadly most likely won't have mind energy in the next few weeks to carry this to completion, so anyone feel free to pick up from there as needed (but I will also aim to loop back if/when I do to see how I can help!)

Grunet avatar Sep 12 '22 21:09 Grunet

It looks like no other SIG has implemented this yet: https://github.com/open-telemetry/opentelemetry-specification/blob/main/spec-compliance-matrix.md#opentracing-compatibility so, my vote is to drop this as a requirement for a GA release.

brettmc avatar Feb 28 '23 05:02 brettmc