-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for gRPC protocol #1623
Merged
Merged
Changes from 40 commits
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
d1a6073
simple starting point to have grpc (unary) requests
rogchap 5b52b07
add gRPC specific metrics and refactor js API
rogchap 2410ef5
add gRPC status codes and use state Dialer
rogchap bb05b01
add support for TLS
rogchap be8f784
manage some of the errors and add the message JSON to the response ob…
rogchap 989f90e
add support for custom tags, headers and timeout
rogchap e08853c
place client in it's own file
rogchap 3d25f99
fix TLS issues and add a timeout on the connection
rogchap 28d5a86
some basic testing of the grpc client
rogchap 5c15a25
add some standard tags
rogchap f431c07
Add Headers/Trailers and seperate the Error message in the response
rogchap 623dbbb
tests for request, headers, reponse, errors, res headers and trailers
rogchap f08a5bf
test for metrics an fix golint messages
rogchap 070b37a
update unix package
rogchap 614be83
fix unix vendoring
rogchap 657db28
fix up the system tags test and deal with some of the lint issues
rogchap 4ce280e
more linting "fixes"
rogchap 8250192
run enumer for new system tags
rogchap c46e726
more linting errors
rogchap f0e9c9c
linter really does not like me
rogchap 5f91c4c
update package to match master
rogchap 907207f
Merge branch 'master' into grpc
rogchap 8ed72d4
vendor clean up
rogchap eaeef08
hack to fix windows file not found error
rogchap 61bd22c
throw on connection errors
rogchap 0dc562b
small API naming change to align more with the gRPC info that is retu…
rogchap f01b8bc
opps; fix test
rogchap 251d259
add time to the metrics
rogchap b48f7b1
add debug information if --http-debug flag is enabled
rogchap fa71eae
fix linting issues
rogchap 539c70a
fix up vendoring and depcricate warnings
rogchap 8b4d384
protoreflect does not like the v2 interface
rogchap 14d81d4
add RPS Limit to Invoke RPCs
rogchap 53695e8
remove grpc_reqs metric as it's obsolte with the trend metrics count …
rogchap cf59c0e
fix for default values and simplify Response object to not use goja v…
rogchap 7cd996a
extend method info and align with grpc lib to include leading slash f…
rogchap 0eb10a4
return error on unknown parameters and better timeout parsing
rogchap 59e98f8
remove string builder for better readability.
rogchap 41ff188
make the channel buffered so that we don't have a goroutine leak
rogchap 1edd999
scope request tags to the context rather than the client
rogchap aa934bc
return errors on param errors rather than continue
rogchap 281aa4a
Merge branch 'master' into grpc
rogchap 8d47d2f
go mod tidy
rogchap d7dc6c1
update vendor modules - I wish this was auto on mod tidy
rogchap 33654b1
fix up some linting errors
rogchap 3fc6969
static check on jsonpb package
rogchap 197a518
refactor to offical protobuf APIv2 and rename API to just invoke
rogchap 28fc5d8
update vendor directory
rogchap 9334e76
ingnore unresolvable files and simplify parsing
rogchap 49ae763
better handling of complex protobuf definitions
rogchap 4f1ca02
resolve the full import graph for the proto files
rogchap 1f62ddf
Merge branch 'master' into grpc
rogchap ab54b18
sync vendor folder
rogchap bb31412
remove rpc type system tag and rps limit
rogchap 32d1812
deal with prealloc lint check
rogchap 6c48328
handle serialisation errors
rogchap 74df86d
scope error handling
rogchap 83c390b
add tests for debug stats and support v1 proto messages
rogchap 0f4baf0
need the APIv1 to support v1 proto messages
rogchap f7482b5
gofumpt-ed
rogchap 4c83ec6
address feedback from team
rogchap 512d34f
Merge branch 'master' into grpc
rogchap 29a4b25
update import to use protocols dir
rogchap File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a grpc and protobuf noob, but while reading up on them it seems like the newer google.golang.org/protobuf` does actually support what protoreflect is used for here? More specifically parsing protobuf definition during runtime and creating messages out of those definitions. Given this and the fact the one is the "standard" implementation, can we actually use that one?
Maybe there are some additional problems that I am not aware of?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is true for the dynamic calls, but unfortunately it does not have the ability to parse the raw *.proto files. Google maintain that the C++ protoc tools is used for the parsing so there are few Go alternatives and protoreflect is the most complete choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm ... I guess nobody has done one that just parsers to the standard lib v2 interface types(if that is possible at all)?
I guess this is the smaller of the big new dependencies that will be included, but still, I would prefer if would've liked it to use less ... if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a parser that does just that; but it's not complete (other priorities) https://github.com/rogchap/protoparser
I opted for a production ready version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mstoykov @na--
So; to make sure we stay "up-to-date", I have refactored a lot of the code to use the Go Protobuf APIv2 and only use this library for the parsing only. This was everything is more standard and if we need to switch out the parser, that can be easily done.
As a side effect I've renamed the API from
Client.invokeRPCto justClient.invoke; it's all RPC (duh) and this lines up with the grpc library under the hood (at the least the Go version)