Reactor Netty: emit actual HTTP client spans spans on connection errors #9063
Conversation
.../opentelemetry/instrumentation/netty/v4/common/internal/client/NettyInstrumentationFlag.java
Outdated
Show resolved
Hide resolved
| ERROR_ONLY, | ||
| DISABLED; | ||
|
|
||
| public static NettyInstrumentationFlag enabledOrErrorOnly(boolean b) { |
There was a problem hiding this comment.
this method name confused me, but I can't think of a better one...
There was a problem hiding this comment.
I couldn't either - if anyone has a suggestion, I'm all ears
There was a problem hiding this comment.
What do you think about calling it fromBoolean(..)?
There was a problem hiding this comment.
What do you think about calling it
fromBoolean(..)?
I'm not too fond of this, because it's a conversion of 2-value type to 3-value type -- and the simple fromBoolean doesn't really say what happens to the 3rd value.
mateuszrzeszutek
force-pushed
the
reactor-netty-part-2
branch
from
3de3908
to
c590ae8
Compare
| false, | ||
| false, | ||
| NettyConnectionInstrumentationFlag.DISABLED, | ||
| NettyConnectionInstrumentationFlag.DISABLED, |
There was a problem hiding this comment.
Why does this one not use the config?
There was a problem hiding this comment.
It's a library instrumentation, I think it just can't implement the connection telemetry stuff.
.../io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/FailedRequestWithUrlMaker.java
Show resolved
Hide resolved
.../io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/FailedRequestWithUrlMaker.java
Outdated
Show resolved
Hide resolved
.../io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/FailedRequestWithUrlMaker.java
Show resolved
Hide resolved
mateuszrzeszutek
force-pushed
the
reactor-netty-part-2
branch
from
8097745
to
e79c943
Compare
| if ("http://localhost:61/".equals(uri.toString()) | ||
| || "https://192.0.2.1/".equals(uri.toString())) { | ||
| return emptySet(); | ||
| attributes.remove(SemanticAttributes.NET_PROTOCOL_NAME); | ||
| attributes.remove(SemanticAttributes.NET_PROTOCOL_VERSION); | ||
| } |
| false, | ||
| connectionTelemetryEnabled | ||
| ? NettyConnectionInstrumentationFlag.ENABLED | ||
| : NettyConnectionInstrumentationFlag.DISABLED, |
There was a problem hiding this comment.
is it not ERROR_ONLY because we capture those spans already in reactor-netty instrumentation?
There was a problem hiding this comment.
Yes, exactly that
Continuation of #8111