Fix span error tagging in grpc server interceptor #980
Conversation
There was a problem hiding this comment.
I don't think this will work this way. Unfortunately, grpc doesn't seem to expose the status for the listener. If we want the status, I think we'd need a more significant change to the instrumentation.
I appreciate the attempt and encourage you to keep trying. I think it's valuable information to collect.
...pc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java
Outdated
Show resolved
Hide resolved
...on/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcServerDecorator.java
Show resolved
Hide resolved
marcoferrer
force-pushed
the
grpc-interceptor-error-tag
branch
from
e78fb9f
to
b547886
Compare
|
@tylerbenson Sorted out ide issues and pushed the proper implementation. Will update tests if everything looks agreeable. |
...pc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java
Show resolved
Hide resolved
...pc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java
Outdated
Show resolved
Hide resolved
...pc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java
Outdated
Show resolved
Hide resolved
| ((TraceScope) scope).setAsyncPropagation(false); | ||
| } | ||
| } catch (final Throwable e) { | ||
| DECORATE.onError(span, e); |
There was a problem hiding this comment.
Any example of an error here that wouldn't be caught elsewhere? (Sorry, I'm not an expert with grpc, so I'm not sure what kinds of exceptions would be thrown here.)
There was a problem hiding this comment.
If the call was already closed by some external hook or client cancellation then this method will throw an IllegalStateException
There was a problem hiding this comment.
Is that a state we really want reported to the span? Is that useful for troubleshooting?
There was a problem hiding this comment.
After some testing it seems that if an error were to occur here it could possibly overwrite the the true source error. Ill remove the catch and push
|
If you'd like to ping me on slack (https://chat.datadoghq.com) to discuss, let me know. I can also try to answer questions to help you fix your IDE setup. It is a pretty complicated project and can be difficult to get working well. |
...pc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java
Outdated
Show resolved
Hide resolved
At the moment there is a discrepancy with how spans are tagged in the client interceptor vs the server interceptor. This PR is meant to synchronize the behavior between the two.