Skip to content
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

Add logging grpc request and response content with grpc-proxy mode #14266

Merged
merged 2 commits into from
Aug 18, 2022

Conversation

stefanmonkey
Copy link

In our test environment, it may be very useful to debug who delete etcd's key with grpc-proxy mode
inspired by https://github.com/grpc-ecosystem/go-grpc-middleware

Signed-off-by: stefanbo [email protected]

grpc_zap.PayloadStreamServerInterceptor(lg, alwaysLoggingDeciderServer),
)
grpcChainUnaryList = append(grpcChainUnaryList,
grpc_ctxtags.UnaryServerInterceptor(),
Copy link
Member

Choose a reason for hiding this comment

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

There is no any option being provided to function grpc_ctxtags.UnaryServerInterceptor, so the o.requestFieldsFunc will be always nil. Accordingly it will do nothing.

Did I miss anything?

Copy link
Author

Choose a reason for hiding this comment

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

@@ -159,6 +163,7 @@ func newGRPCProxyStartCommand() *cobra.Command {
// experimental flags
cmd.Flags().BoolVar(&grpcProxyEnableOrdering, "experimental-serializable-ordering", false, "Ensure serializable reads have monotonically increasing store revisions across endpoints.")
cmd.Flags().StringVar(&grpcProxyLeasing, "experimental-leasing-prefix", "", "leasing metadata prefix for disconnected linearized reads.")
cmd.Flags().BoolVar(&grpcProxyEnableLogging, "experimental-grpc-logging", false, "logging all grpc request and response")
Copy link
Member

Choose a reason for hiding this comment

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

--experimental-enable-grpc-debug ?

Copy link
Author

Choose a reason for hiding this comment

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

I am Ok with that

@ahrtr
Copy link
Member

ahrtr commented Jul 28, 2022

Two comments:

  1. Please add a changelog item;
  2. Please provide your test result/examples.

@stefanmonkey
Copy link
Author

stefanmonkey commented Jul 29, 2022

Environment

etcd server: 192.168.56.11:23790 in vagrant ubuntu
etcd grpc-proxy: localhost:2379 in macos

Start scripts

  • etcd server
export REGISTRY=quay.io/coreos/etcd

docker run -d \
  -p 23790:23790 \
  -p 23800:23800 \
  --name etcd ${REGISTRY}:latest \
  /usr/local/bin/etcd \
  --data-dir=/etcd-data --name node1 \
  --initial-advertise-peer-urls http://0.0.0.0:23800 --listen-peer-urls http://0.0.0.0:23800 \
  --advertise-client-urls http://0.0.0.0:23790 --listen-client-urls http://0.0.0.0:23790 \
  --initial-cluster node1=http://0.0.0.0:23800
  • etcd grpc-proxy
./etcd grpc-proxy start \
--debug \
--experimental-enable-grpc-logging=true \
--endpoints=192.168.56.11:23790 \
--listen-addr=127.0.0.1:2379

Test process

./etcdctl put greeting "Hello, world"
# logging content
{"level":"info","ts":"2022-07-29T10:14:41.890+0800","caller":"zap/payload_interceptors.go:131","msg":"server request payload logged as grpc.request.content field","system":"grpc","span.kind":"server","grpc.service":"etcdserverpb.KV","grpc.method":"Put","peer.address":"127.0.0.1:54276","grpc.request.content":{"msg":{"key":"Z3JlZXRpbmc=","value":"SGVsbG8sIHdvcmxk"}}}
{"level":"info","ts":"2022-07-29T10:14:41.891+0800","caller":"zap/payload_interceptors.go:131","msg":"server response payload logged as grpc.response.content field","system":"grpc","span.kind":"server","grpc.service":"etcdserverpb.KV","grpc.method":"Put","peer.address":"127.0.0.1:54276","grpc.response.content":{"msg":{"header":{"clusterId":"12526192129151561082","memberId":"3877455311180318034","revision":"2","raftTerm":"2"}}}}

./etcdctl get greeting
# logging content
{"level":"info","ts":"2022-07-29T10:16:24.250+0800","caller":"zap/payload_interceptors.go:131","msg":"server request payload logged as grpc.request.content field","system":"grpc","span.kind":"server","grpc.service":"etcdserverpb.KV","grpc.method":"Range","peer.address":"127.0.0.1:54297","grpc.request.content":{"msg":{"key":"Z3JlZXRpbmc="}}}
{"level":"info","ts":"2022-07-29T10:16:24.251+0800","caller":"zap/payload_interceptors.go:131","msg":"server response payload logged as grpc.response.content field","system":"grpc","span.kind":"server","grpc.service":"etcdserverpb.KV","grpc.method":"Range","peer.address":"127.0.0.1:54297","grpc.response.content":{"msg":{"header":{"clusterId":"12526192129151561082","memberId":"3877455311180318034","revision":"2","raftTerm":"2"},"kvs":[{"key":"Z3JlZXRpbmc=","createRevision":"2","modRevision":"2","version":"1","value":"SGVsbG8sIHdvcmxk"}],"count":"1"}}}

summary

In Test deployment, with grpc-proxy before etcdserver, --experimental-enable-grpc-logging=true will help a lot to debug

@@ -56,6 +56,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0).
- Add [v3 discovery](https://github.com/etcd-io/etcd/pull/13635) to bootstrap a new etcd cluster.
- Add [field `storage`](https://github.com/etcd-io/etcd/pull/13772) into the response body of endpoint `/version`.
- Add [`etcd --max-concurrent-streams`](https://github.com/etcd-io/etcd/pull/14169) flag to configure the max concurrent streams each client can open at a time, and defaults to math.MaxUint32.
- Add [`etcd grpc-proxy --experimental-enable-grpc-debug`](https://github.com/etcd-io/etcd/pull/14266) flag to logging all grpc request and response
Copy link
Member

Choose a reason for hiding this comment

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

minor comments, requests and responses

Copy link
Author

Choose a reason for hiding this comment

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

has updated, please check

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

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

lgtm but please see inline comment related to naming duplication cc @ahrtr . Thanks @stefanmonkey

server/etcdmain/grpc_proxy.go Outdated Show resolved Hide resolved
…ponse content in grpc-proxy mode In our test environment, it may be very useful to debug who delete etcd's key with grpc-proxy mode

inspired by https://github.com/grpc-ecosystem/go-grpc-middleware

Signed-off-by: stefanbo <[email protected]>

Update flag name

1. add changelog
2. update flag name to `experimental-enable-grpc-debug`

Signed-off-by: stefan bo <[email protected]>

Update CHANGELOG-3.6.md

Signed-off-by: stefan bo <[email protected]>

change flag name

Signed-off-by: stefan bo <[email protected]>
@ahrtr
Copy link
Member

ahrtr commented Aug 15, 2022

All comments resolved, please @spzala take another look, thx

@ahrtr ahrtr merged commit 1851316 into etcd-io:main Aug 18, 2022
@stefanmonkey stefanmonkey deleted the feat/grpc-logging branch September 15, 2022 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants