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

Fix Timer stopping in http.send #7007

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

lukyer
Copy link
Contributor

@lukyer lukyer commented Sep 9, 2024

Fixes incorrect Timer measuring in case of error. Without calling Stop() method when e.g. eval_cancel_error happens the last delta is not added to the accumulated Timer value.

I discovered it by investigating eval_cancel_error when the caller gave up but according to OPA metrics it looked like http.send did not cause it (I would expect near 10s value in timer_rego_builtin_http_send_ns too):

{"counter_rego_builtin_http_send_interquery_cache_hits":1,
"counter_server_query_cache_hit":1,
"timer_rego_builtin_http_send_ns":124580,
"timer_rego_input_parse_ns":4771,
"timer_rego_query_eval_ns":9770617400,
"timer_server_handler_ns":9770659804} 

Fixes incorrect `Timer` measuring in case of error. Without calling `Stop()` method when e.g. `eval_cancel_error` happens the last delta is not added to the accumulated `Timer` value.

I discovered it by investigating `eval_cancel_error` when the caller gave up but according to OPA metrics it looked like `http.send` did not cause it (I would expect near 10s value in `timer_rego_builtin_http_send_ns` too):
```
{"counter_rego_builtin_http_send_interquery_cache_hits":1,"counter_server_query_cache_hit":1,"timer_rego_builtin_http_send_ns":124580,"timer_rego_input_parse_ns":4771,"timer_rego_query_eval_ns":9770617400,"timer_server_handler_ns":9770659804} 
```

Signed-off-by: lukyer <lukyer@gmail.com>
topdown/http.go Outdated Show resolved Hide resolved
Signed-off-by: lukyer <lukyer@gmail.com>
@lukyer lukyer marked this pull request as ready for review September 9, 2024 13:08
@ashutosh-narkar ashutosh-narkar merged commit d250ea4 into open-policy-agent:main Sep 9, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants