robinhood / faust

Python Stream Processing
Other
6.75k stars 534 forks source link

Opentracing 2.0.0 support #528

Open jwthomp opened 4 years ago

jwthomp commented 4 years ago

Checklist

Steps to reproduce

Running the above commands demonstrates the inability to install datadogs ddtrace with opentracing support.

Expected behavior

Expected ddtrace[opentracing] dependency to be added.

Actual behavior

ddtrace[opentracing] did not install due to version requirement conflicts between ddtrace and opentracing.

Full traceback

Jeff-Thompson:tmp $ poetry add faust
Creating virtualenv tmp--vfS0y24-py3.7 in /Users/jeffreythompson/Library/Caches/pypoetry/virtualenvs
Using version ^1.10.1 for faust

Updating dependencies
Resolving dependencies... (1.4s)

Writing lock file

Package operations: 23 installs, 0 updates, 0 removals

  - Installing idna (2.8)
  - Installing multidict (4.7.4)
  - Installing async-timeout (3.0.1)
  - Installing attrs (19.3.0)
  - Installing chardet (3.0.4)
  - Installing six (1.14.0)
  - Installing yarl (1.4.2)
  - Installing aiohttp (3.6.2)
  - Installing colorlog (4.1.0)
  - Installing kafka-python (1.4.7)
  - Installing mypy-extensions (0.4.3)
  - Installing python-dateutil (2.8.1)
  - Installing typing-extensions (3.7.4.1)
  - Installing aiohttp-cors (0.7.0)
  - Installing click (7.0)
  - Installing colorclass (2.2.0)
  - Installing croniter (0.3.31)
  - Installing mode (4.3.0)
  - Installing opentracing (1.3.0)
  - Installing robinhood-aiokafka (1.1.3)
  - Installing terminaltables (3.1.0)
  - Installing venusian (1.2.0)
  - Installing faust (1.10.1)
Jeff-Thompson:tmp $ poetry add ddtrace[opentracing]
Using version ^0.33.0 for ddtrace

Updating dependencies
Resolving dependencies... (0.2s)

[SolverProblemError]
Because no versions of ddtrace match >0.33.0,<0.34.0
 and ddtrace (0.33.0) depends on opentracing (>=2.0.0), ddtrace (>=0.33.0,<0.34.0) requires opentracing (>=2.0.0).
And because faust (1.10.1) depends on opentracing (>=1.3.0,<2.0.0)
 and no versions of faust match >1.10.1,<2.0.0, ddtrace (>=0.33.0,<0.34.0) is incompatible with faust (>=1.10.1,<2.0.0).
So, because tmp depends on both faust (^1.10.1) and ddtrace (^0.33.0), version solving failed.

Versions

axsaucedo commented 4 years ago

We also have a strong dependency on Opentracing > 2.0.0 as well, and several of our production use-cases have requirements for libraries to be updated to a recent library, so we require this second level dependency to also be met.

For us it would even be enough to remove the strong requirement and just disable opentracing functionality if a non-compatible version is available.

We'd be happy to explore adding a PR to implement the above - alternatively I'd be interested to understand the complexity of supportin OpenTracing 2.0.0 (which would be ideal).

axsaucedo commented 4 years ago

@ask wondering if there could be any pointers on what's the best way to approach this - I'd be happy to add a PR to contribute something. Initially it could be by moving OpenTracing to extras_require, or alternatively by disabling tracing if OpenTracing < 2.0 is not installed with a message. Alternativelly happy to also explore what would be the way to add support for OpenTracing 2.0, and whether it's possible to keep compability for both. Would be happy to dive into this if it's something you'd be open to review as PR contribution.

ask commented 4 years ago

@axsaucedo I've been meaning to upgrade to Opentracing v2 so that would be great! Apparently this version supports keeping track of the current span also, so may be able to remove some code.

ask commented 4 years ago

From initial test it looks like the opentracing 2.x works by just upgrading the requirement.

Faust also has some custom tracing support, but we largely depend on a non-open source library. I'm not sure if that part still works after upgrading, but what we want to do is to eventually move our tracing over to the official one.

axsaucedo commented 4 years ago

That's great news @ask - we also recently moved to Opentracing 2.0 because of the new benefits it provides. Great to see it's not part of the milestone. Happy to test this on the dev build when it's ready as well.

hamima-halim commented 4 years ago

Quick bump--will this issue be out soon? This upgrade would also be great for us as we use 2.x in our production.

hamima-kensho commented 3 years ago

Bumping this issue again, would appreciate if this got a pr.