Capture HTTP2 headers in netty gRPC client/server #158
Conversation
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
| } | ||
| } | ||
|
|
||
| static class Utils_convertClientHeaders_Advice { |
There was a problem hiding this comment.
nit: GrpcUtils maybe?
There was a problem hiding this comment.
The target class is called Utils therefore the Utils name.
| isMethod().and(named("convertClientHeaders")).and(takesArguments(6)), | ||
| Utils_convertClientHeaders_Advice.class.getName()); | ||
| transformers.put( | ||
| isMethod().and(named("convertHeaders")).and(takesArguments(1)), |
There was a problem hiding this comment.
I think this is called multiple times from different flows in code.. How are we keeping track when to use and when not to use?
There was a problem hiding this comment.
correct we do not keep track of objects where should be added. I want to avoid using TLS and global map to do this as it is done in the old impl.
The HTTP2 headers are added under different keys than the gRPC impl uses e.g. :method.ht to do not collide with user app. The implementation does not change any objects it just adds more data to metadata object passed around I think this is acceptable also the added headers are not added to the wire.
For completion: this advice does not change client response headers since those headers do not have the HTTP2 we are using (see the comment in the code). If the server side will cause any issues we could remove added headers in the interceptor before passing it to user define interceptors. The removal seems expensive so I would rather skip it for now.
| @Advice.Argument(3) Object authority, | ||
| @Advice.Argument(4) Object method) { | ||
|
|
||
| Span currentSpan = Java8BytecodeBridge.currentSpan(); |
There was a problem hiding this comment.
This method can be called when a span does not exist right? We need to add these attributes only when some span has been started.. Are we tracking that? If yes, then span can be null -- need to handle that..
There was a problem hiding this comment.
span is never null. It is either DefaultSpan or recording events span.
|
|
||
| if (http2Headers.authority() != null) { | ||
| metadata.put( | ||
| GrpcSemanticAttributes.AUTHORITY_METADATA_KEY, http2Headers.authority().toString()); |
There was a problem hiding this comment.
we are changing original user metadata object -- need to be very careful if this does not break anything -- especially since this method is called from different flows in code..
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
pavolloffay
changed the title
Signed-off-by: Pavol Loffay p.loffay@gmail.com
Updates #109