-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
examples/features/csm_observability: use helloworld client and server instead of echo client and server #7945
examples/features/csm_observability: use helloworld client and server instead of echo client and server #7945
Conversation
fe9087f
to
690c2c1
Compare
addr string | ||
} | ||
|
||
func (s *echoServer) UnaryEcho(_ context.Context, req *pb.EchoRequest) (*pb.EchoResponse, error) { | ||
return &pb.EchoResponse{Message: fmt.Sprintf("%s (from %s)", req.Message, s.addr)}, nil | ||
// SayHello implements helloworld.GreeterServer |
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.
SayHello
is a method, it's inaccurate to say that it implements an interface. I believe this comment should be on server
instead.
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.
Actually, this is what we have in every example. I just took it from helloworld and observability example
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.
Did you manually run the client and server to verify they work?
yes, i checked the output on server and client and prometheous |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7945 +/- ##
==========================================
- Coverage 82.15% 82.03% -0.12%
==========================================
Files 381 381
Lines 38539 38539
==========================================
- Hits 31660 31615 -45
- Misses 5574 5600 +26
- Partials 1305 1324 +19 |
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.
Looks good, should complete testing in k8s before merging.
8d08898
to
6db15dc
Compare
b599957
to
127f91c
Compare
RELEASE NOTES: