HttpTraceContext::extract() fails on traceparent == null

[prydin]

io.opentelemetry.trace.propagation.HttpTraceContext::extract() can be used to construct a SpanContext from HTTP headers/request attributes. This is very useful when instrumenting servlets, for example.

However, this method fails on an IllegalArgumentException when the traceparent attribute can't be located. I don't think this exception should be called, since root spans should not have this attribute set. I believe the correct behavior should just be to copy the null to the constructed SpanContext.

[prydin]

Also, catching the exception and trying to manually set the parent to null through Span.Builder::setParent() doesn't work either, since it doesn't accept null parameters.

EDIT: My bad. There's setNoParent()

[carlosalberto]

I agree on this, and I think we should try to not throw exceptions for non-fatal cases.

[DotSpy]

I will try fix it, please assign me

[DotSpy]

@pavolloffay how do u think should we return null here (

opentelemetry-java/api/src/main/java/io/opentelemetry/trace/propagation/HttpTraceContext.java

Line 113 in 7b601b1

throw new IllegalArgumentException("Traceparent not present");

) or SpanContext.INVALID ?
Also i think that it's kinda heavy method, should i also refactor it?

[jkwatson]

I'd love to see some refactoring in there, @DotSpy !

[carlosalberto]

My advise is to keep different PRs for any refactoring (given it's not only about code style, but actual improved readability) and for the null-handling cases.

[DotSpy]

@jkwatson @carlosalberto how do u think what should i return instead of throwing:
null - but here in Java Doc it said

opentelemetry-java/context_prop/src/main/java/io/opentelemetry/context/propagation/HttpTextFormat.java

Line 98 in bd4f373

* @return the extracted value, never {@code null}.

or SpanContext.INVALID

[jkwatson]

I'm not a Tracing expert, but I think INVALID seems like a great answer.

[carlosalberto]

SpanContext.INVALID definitely.

[DotSpy]

I think we can close this one because it's merged

[jkwatson]

Resolved by #782