mchlvl / starlette-zipkin

Zipkin client for asgi. Compatible with Starlette Framework and Jaeger tracing server
10 stars 5 forks source link

Features/add a trace #21

Closed mardiros closed 2 years ago

mardiros commented 2 years ago

Hi @mchlvl

I'd like to improve the developer experience of using the starlette-zipkin middleware by adding a trace decorator/contextmanager.

I've added a few tests to get the idea.

If you are ok with it, I can update the readme as well.

codecov-commenter commented 2 years ago

Codecov Report

Merging #21 (ef2b42e) into master (559f977) will increase coverage by 0.70%. The diff coverage is 90.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
+ Coverage   85.51%   86.21%   +0.70%     
==========================================
  Files           6        8       +2     
  Lines         214      283      +69     
==========================================
+ Hits          183      244      +61     
- Misses         31       39       +8     
Impacted Files Coverage Δ
starlette_zipkin/middleware.py 84.15% <83.33%> (-2.25%) :arrow_down:
starlette_zipkin/trace.py 88.60% <88.60%> (ø)
starlette_zipkin/__init__.py 100.00% <100.00%> (ø)
starlette_zipkin/config.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 559f977...ef2b42e. Read the comment docs.

mardiros commented 2 years ago

@mchlvl,

I finally update the README, add an examples directory with an example with starlette (an example with FastAPI could be great too, but out of scope).

I have done some refactoring to avoid circular reference ( f342315 ).

Finally, I think that the root trace could also be handle by the new trace context manager but it will introduce more changes that I can do later if you are ok with the design.

mchlvl commented 2 years ago

@mardiros Thanks for another round of improvements, it does feel a bit neater! Happy to put this in and release.

Also, given that you have now co-written most of the code, I've sent an invite to collaborate on the repo, such that I am not the only blocker in putting improvements out!

mardiros commented 2 years ago

@mchlvl ,

Thanks for the invitation, I just got a beautifull 500 error, and now the invitation is invalid 🥲

image

mchlvl commented 2 years ago

maybe it worked? image