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

Should we remove SpanContextNotFound? #49

Closed
jcchavezs opened this issue Feb 9, 2018 · 1 comment · Fixed by #56
Closed

Should we remove SpanContextNotFound? #49

jcchavezs opened this issue Feb 9, 2018 · 1 comment · Fixed by #56

Comments

@jcchavezs
Copy link
Collaborator

Problem

After doing a couple implementation of OpenTracing, I did not have the necessity of using the SpanContextNotFound exception. More over, the flow when using this exception is cumbersome:

try {
    $context = $tracer->extract(...);
} catch(Exception $e) {
    $context = SpanContext::createAsRoot();
}

whereas returning a null would be:

$context = $tracer->extract(...);

if ($context === null) {
    ... // build root context
}

Finally, the SpanContextNotFound does not add additional value to the failure, so returning a null is good enough.

Proposal

Get rid of the exception and return null in such cases.

Ping @tedsuo @yurishkuro @beberlei @felixfbecker @lvht

@yurishkuro
Copy link
Member

+1, especially if childOf() can accept null, then you don't even need the null check.

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

Successfully merging a pull request may close this issue.

2 participants