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

[internal] grpc_util: add layer to capture network-level timing data #15978

Merged
merged 3 commits into from
Jun 29, 2022

Conversation

tdyas
Copy link
Contributor

@tdyas tdyas commented Jun 28, 2022

Our REAPI client uses concurrency_limit to ensure only a certain maximum number of client calls run concurrently. This causes any timing metrics computed outside of that concurrency limit to include the queuing time in addition to the actual network RPC time.

This PR defines a layer below the concurrency limit that will capture just the network RPC time. Timings are captured based on the URI path which maps to a specific observation metric. Units are microseconds.

Tom Dyas added 3 commits June 28, 2022 17:16
@tdyas tdyas force-pushed the grpc_low_level_timing_capture branch from 8c40211 to ee0e5e3 Compare June 28, 2022 21:36
@tdyas tdyas added the category:internal CI, fixes for not-yet-released features, etc. label Jun 28, 2022
@tdyas tdyas marked this pull request as ready for review June 28, 2022 21:38
@tdyas
Copy link
Contributor Author

tdyas commented Jun 28, 2022

Commits can be reviewed individually.

@tdyas
Copy link
Contributor Author

tdyas commented Jun 28, 2022

Metrics were successfully generated:
Screenshot of metrics

@tdyas tdyas changed the title [internal] grpc_util: add layer to capture low-level timing data [internal] grpc_util: add layer to capture network-level timing data Jun 29, 2022
@tdyas tdyas merged commit 6d805d2 into pantsbuild:main Jun 29, 2022
@tdyas tdyas deleted the grpc_low_level_timing_capture branch June 29, 2022 01:37
tdyas pushed a commit to tdyas/pants that referenced this pull request Jun 29, 2022
…antsbuild#15978)

Our REAPI client uses `concurrency_limit` to ensure only a certain maximum number of client calls run concurrently. This causes any timing metrics computed outside of that concurrency limit to include the queuing time in addition to the actual network RPC time.

This PR defines a layer below the concurrency limit that will capture just the network RPC time. Timings are captured based on the URI path which maps to a specific observation metric. Units are microseconds.
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
tdyas pushed a commit that referenced this pull request Jun 29, 2022
…(Cherry-pick of #15978) (#15999)

Our REAPI client uses `concurrency_limit` to ensure only a certain maximum number of client calls run concurrently. This causes any timing metrics computed outside of that concurrency limit to include the queuing time in addition to the actual network RPC time.

This PR defines a layer below the concurrency limit that will capture just the network RPC time. Timings are captured based on the URI path which maps to a specific observation metric. Units are microseconds.
@stuhood stuhood mentioned this pull request Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants