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

add grpc unary interceptor for trace #12

Closed
wants to merge 1 commit into from
Closed

Conversation

aoliang
Copy link

@aoliang aoliang commented Feb 10, 2021

No description provided.

@wu-sheng
Copy link
Member

So, streaming is not supported in this PR?

@wu-sheng wu-sheng added this to the 0.6.0 milestone Feb 10, 2021
@wu-sheng wu-sheng added the enhancement New feature or request label Feb 10, 2021
@aoliang
Copy link
Author

aoliang commented Feb 13, 2021

So, streaming is not supported in this PR?

yes. not support stream. stream interceptor not work well with trace, no context param.

@wu-sheng
Copy link
Member

So, streaming is not supported in this PR?

yes. not support stream. stream interceptor not work well with trace, no context param.

I am feeling confused. Streaming of gRPC java supported, and basically in the implementation level, unary based on streaming too.

grpc/README.md Outdated
## Installation

```bash
go get -u github.com/aoliang/go2sky-plugins/grpc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
go get -u github.com/aoliang/go2sky-plugins/grpc
go get -u github.com/SkyAPM/go2sky-plugins/grpc

Copy link
Author

Choose a reason for hiding this comment

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

done

README.md Outdated
@@ -12,3 +12,4 @@ The plugins of [go2sky](https://github.com/SkyAPM/go2sky)
1. [go-resty](resty/README.md)
1. [go-micro](micro/README.md)
1. [go-restful](go-restful/README.md)
1. [grpc](grpc/README.md)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. [grpc](grpc/README.md)
1. [grpc(unary)](grpc/README.md)

Copy link
Author

Choose a reason for hiding this comment

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

done

grpc/README.md Outdated

## Note

just for unary server and unary client, not support streaming.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
just for unary server and unary client, not support streaming.
This plugin works just for gRPC server and client in unary mode, streaming has not been supported.

Copy link
Author

Choose a reason for hiding this comment

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

done

@aoliang
Copy link
Author

aoliang commented Feb 25, 2021

So, streaming is not supported in this PR?

yes. not support stream. stream interceptor not work well with trace, no context param.

I am feeling confused. Streaming of gRPC java supported, and basically in the implementation level, unary based on streaming too.

  1. context put into thread local in java. golang not thread local。interceptor has no context param。
  2. java agent plugin also enhance blockingUnaryCall。not Server streamingClient streamingBidirectional streaming. and no meaning to do trace for streaming, because maybe no response from server or continous response message from server.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
3 participants