Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How can I create client root span? #35

Closed
dmitry-prokopchenkov opened this issue May 14, 2017 · 10 comments
Closed

How can I create client root span? #35

dmitry-prokopchenkov opened this issue May 14, 2017 · 10 comments

Comments

@dmitry-prokopchenkov
Copy link
Contributor

'sr' and 'ss' annotations are always added to root span in logging_helper.py:150. It's not correctly for client root spans.

@codefromthecrypt
Copy link

sounds like a legit limitation in the current code.

@dmitry-prokopchenkov
Copy link
Contributor Author

Guys, would you mind if I create a pull request and add this ability by myself?

@bplotnick
Copy link
Contributor

@dmitry-prokopchenkov by all means!! We love pull requests! I'm actually not sure how this would look, so i'm interested to see what you might come up with 😄 . Let us know if you need a hand.

P.S. For some historical context, the reason this limitation exists is that py_zipkin was formed out of our initial project pyramid_zipkin, and in fact is still the main way we use py_zipkin. For us, everything starts with a service request. But I totally see why it would be useful to be able to start with a client span.

@dmitry-prokopchenkov
Copy link
Contributor Author

ok, thanks, I will give it a try when I have some time

@bplotnick
Copy link
Contributor

closed via #49

@kaisen
Copy link
Member

kaisen commented Jun 30, 2017

@dmitry-prokopchenkov your change is included in the tagged v0.8.2 release :)

@dmitry-prokopchenkov
Copy link
Contributor Author

@kaisen sorry, it was better to not change the release version, yes?

@bplotnick
Copy link
Contributor

@dmitry-prokopchenkov i'm not sure what you're asking. are you talking about why we bumped the patch version rather than the minor version? we generally use semver at Yelp, but releases <1.0.0 have unstable APIs, so the usual semver rules are much looser and it's kind of a judgement call.

@kaisen
Copy link
Member

kaisen commented Jul 3, 2017

@dmitry-prokopchenkov did you mean that it would have been better if you didn't change the release version in your PR? It doesn't matter to me :)

@dmitry-prokopchenkov
Copy link
Contributor Author

@kaisen , @bplotnick ok, thanks for the explanations. I am looking forward for answer on my question about batch span sending. This is that I might add quickly enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants