-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Remove support for gRPC-Plugin #5388
Remove support for gRPC-Plugin #5388
Conversation
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
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.
- please delete code, not comment out
- changes look like in the right direction, but they don't compile
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5388 +/- ##
==========================================
+ Coverage 95.30% 95.51% +0.21%
==========================================
Files 334 331 -3
Lines 16231 16107 -124
==========================================
- Hits 15469 15385 -84
+ Misses 582 550 -32
+ Partials 180 172 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
|
||
// ServeWithGRPCServer creates a plugin configuration using the implementation of StoragePlugin and | ||
// function to create grpcServer, and then serves it. | ||
func ServeWithGRPCServer(services *shared.PluginServices, grpcServer func([]grpc.ServerOption) *grpc.Server, |
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.
Is this removing the sidecar model or removing grpc plugin in general? What is the new correct way to start a remote grpc server if the StorageGRPCPlugin is also removed?
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.
Jaeger still supports and will support Remote Storage API. You can write your gRPC server however you wish, no dependency on the Jaeger code is required.
Which problem is this PR solving?
Description of the changes
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test