Skip to content
This repository was archived by the owner on Oct 3, 2023. It is now read-only.

Add RPC constants started_rpcs.#120

Merged
songy23 merged 2 commits intocensus-instrumentation:masterfrom
songy23:rpc-constants
Jun 4, 2018
Merged

Add RPC constants started_rpcs.#120
songy23 merged 2 commits intocensus-instrumentation:masterfrom
songy23:rpc-constants

Conversation

@songy23
Copy link
Copy Markdown
Contributor

@songy23 songy23 commented Jun 2, 2018

started_rpcs is required by gRPC and is not covered by our current constants. See https://github.com/grpc/grpc-java/pull/4494/files#r191610513.

Comment thread stats/gRPC.md Outdated
| grpc.io/client/received_bytes_per_rpc | grpc.io/client/received_bytes_per_rpc | distribution | grpc_client_method |
| grpc.io/client/roundtrip_latency | grpc.io/client/roundtrip_latency | distribution | grpc_client_method |
| grpc.io/client/completed_rpcs | grpc.io/client/roundtrip_latency | count | grpc_client_method, grpc_client_status |
| grpc.io/client/started_rpcs | grpc.io/client/started_rpcs | count | grpc_client_method, grpc_client_status |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Status should not be exposed by started counts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see, done.

Comment thread stats/gRPC.md Outdated
| grpc.io/server/sent_bytes_per_rpc | grpc.io/server/sent_bytes_per_rpc | distribution | grpc_server_method |
| grpc.io/server/server_latency | grpc.io/server/server_latency | distribution | grpc_server_method |
| grpc.io/server/completed_rpcs | grpc.io/server/server_latency | count | grpc_server_method, grpc_server_status |
| grpc.io/server/started_rpcs | grpc.io/server/started_rpcs | count | grpc_server_method, grpc_server_status |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ditto

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@songy23
Copy link
Copy Markdown
Contributor Author

songy23 commented Jun 4, 2018

Going to merge this PR by EOD today if there are no further concerns/questions. Thanks for review!

@songy23
Copy link
Copy Markdown
Contributor Author

songy23 commented Jun 4, 2018

Thanks!

@songy23 songy23 merged commit 342a2a0 into census-instrumentation:master Jun 4, 2018
@songy23 songy23 deleted the rpc-constants branch June 27, 2018 02:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants