Having no tracing headers should not be an error. #43
Conversation
basictracer/text_propagator.py
Outdated
| def extract(self, carrier): | ||
| return self._extract(carrier) | ||
|
|
||
| def _extract(self, carrier, allow_zero_count=False): # noqa |
There was a problem hiding this comment.
I agree with your PR description - no incoming trace context should result in None. I don't think allow_zero_count customization is necessary.
There was a problem hiding this comment.
That was my original thought but there is a test that explicitly checks this case, which is when I went for the HTTP specific approach. I'm happy either way.
There was a problem hiding this comment.
yeah, I think it's ok to change that test. I think what it should be checking is if N of fields is 0 < N < 3, then we expect an error because the headers are malformed, but if it's ==0 then it's ok.
There was a problem hiding this comment.
btw, please add a test for ==0 case.
|
@yurishkuro Thanks for looking at this. Not sure what is going on with Python 3.5 in the CI, it seems related to setting up the environment and not the change. |
|
I just removed 3.5, can you rebase? |
It is quite typical for services at the edge to receive requests with no tracing headers. This case shouldn't throw an exception but rather return None, as we are starting the root span. This is consistent with the documented behavior of extract on Tracer: https://github.com/opentracing/opentracing-python/blob/946f763510a8c26a66d5c53a9c4d0c6027cb9129/opentracing/tracer.py#L214
Rebased. |
|
Thanks! |
It is quite typical for services at the edge to receive requests with no
tracing headers. This case shouldn't throw an exception but rather return
None, as we are starting the root span. This is consistent with the
documented behavior of extract on Tracer:
https://github.com/opentracing/opentracing-python/blob/946f763510a8c26a66d5c53a9c4d0c6027cb9129/opentracing/tracer.py#L214