Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Clarify that Tracer.extract can return null #203

Closed
wants to merge 1 commit into from

Conversation

mabn
Copy link
Contributor

@mabn mabn commented Oct 6, 2017

According to #168 in practice the tracer implementations return null when SpanContext is not found in the carrier. This change documents this behaviour.

It also adds a dependency on com.google.code.findbugs:jsr305 but it's optional, it is not listed in the pom and client applications do not need to depend on jsr305.

Alt text

Compiling the code above with:

javac -Werror -Xlint:all -cp \
/Users/mabn/.m2/repository/io/opentracing/opentracing-api/0.30.1-SNAPSHOT/opentracing-api-0.30.1-SNAPSHOT.jar:\
/Users/mabn/.m2/repository/io/opentracing/opentracing-noop/0.30.1-SNAPSHOT/opentracing-noop-0.30.1-SNAPSHOT.jar:\
/Users/mabn/.m2/repository/io/opentracing/opentracing-util/0.30.1-SNAPSHOT/opentracing-util-0.30.1-SNAPSHOT.jar \
Main.java

procudes no warnings.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.09% when pulling 0ff87b8 on futuresimple:nullable_extract into e000eb0 on opentracing:master.

@mabn mabn mentioned this pull request Oct 6, 2017
</properties>

<dependencies>
<dependency>
<groupId>com.google.code.findbugs</groupId>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this in a separate PR? It might apply also at different places

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to split adding jsr305 and marking Tracer.extract into two separate PRs?

@mabn
Copy link
Contributor Author

mabn commented Jun 24, 2019

I'll close this PR as I haven't looked into this for a while.

@mabn mabn closed this Jun 24, 2019
@whiskeysierra
Copy link

I'd still like to see this happening, tbh.

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

Successfully merging this pull request may close these issues.

4 participants