added error count of failed requests from gateway service to query service #128
Conversation
|
adding some further commits based on the feedback will reopen the PR later. |
…or_count_gateway-service merging with main branch.
…n/gateway-service into error_count_gateway-service
Codecov Report
@@ Coverage Diff @@
## main #128 +/- ##
============================================
- Coverage 79.56% 79.26% -0.31%
- Complexity 1215 1216 +1
============================================
Files 113 113
Lines 5472 5503 +31
Branches 446 446
============================================
+ Hits 4354 4362 +8
- Misses 920 943 +23
Partials 198 198
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
|
||
| private Counter serviceResponseErrorCounter; | ||
| private Counter serviceResponseSuccessCounter; | ||
| private static final String ERROR_COUNTER_NAME = "hypertrace.gateway.response.errors"; |
There was a problem hiding this comment.
Shall we have single metrics instead of two with status for fail/success? This way, you can check the call rate too.
e.g
hypertrace_gateway_requests_status {"code"="fail"}
hypertrace_gateway_requests_status {"code"="success"}
Any suggestions code or status or error?
hypertrace_gateway_requests_status {"error"="true"}
hypertrace_gateway_requests_status {"error"="false"}
Secondly, shall we have the same naming convention across services? I see that here - hypertrace/query-service#138 - we have a different convention.
So, should we go with :
hypertrace_gateway_service_requests_status ?
And in PR - hypertrace/query-service#138,
hypertrace.query.service.request.status?
There was a problem hiding this comment.
Shall we have single metrics instead of two with status for fail/success?
Yes makes sense.
Below looks good:
hypertrace.gateway.service.requests.status {"error"="true"}
hypertrace.gateway.service.requests.status {"error"="false"}
shall we have the same naming convention across services?
Yes my bad, keeping the same naming convention across both the services like - hypertrace.<servicename>.service.requests.status
Pushing the above changes.
|
|
||
| private void initMetrics() { | ||
| serviceResponseErrorCounter = | ||
| PlatformMetricsRegistry.registerCounter(ERROR_COUNTER_NAME, ImmutableMap.of()); |
There was a problem hiding this comment.
If we go with the above metrics - #128 (comment), you may have to register a counter with
serviceResponseErrorCounter = PlatformMetricsRegistry.registerCounter("hypertrace.gateway.requests.status", ImmutableMap.of("code", "fail"))
And,
serviceResponseSuccessCounter =
PlatformMetricsRegistry.registerCounter("hypertrace.gateway.requests.status", ImmutableMap.of("code", "success"))
There was a problem hiding this comment.
Yes. Making the change.
|
@laxman-traceable @ravisingal Do you have any suggestions on metrics? |
kotharironak
requested review from
laxman-traceable,
laxmanchekka and
ravisingal
| private static final String REQUEST_TIMEOUT_CONFIG_KEY = "request.timeout"; | ||
| private static final int DEFAULT_REQUEST_TIMEOUT_MILLIS = 10000; | ||
|
|
||
| private Counter serviceResponseErrorCounter; |
There was a problem hiding this comment.
nit:
serviceResponseErrorCounter -> requestStatusErrorCounter
serviceResponseSuccessCounter -> requestStatusSuccessCounter
…rvice (hypertrace#128) * added error count of failed requests from gateway service to query service * added success metric in the gateway service * added success metric in gateway service * spotless apply fix for the github actions * adressing the PR comments 1 * spotlessApply fix * changed the metric tags * adressing the PR comments 2
…rvice (hypertrace#128) * added error count of failed requests from gateway service to query service * added success metric in the gateway service * added success metric in gateway service * spotless apply fix for the github actions * adressing the PR comments 1 * spotlessApply fix * changed the metric tags * adressing the PR comments 2 cherry-picking gateway service error counts
…rvice (hypertrace#128) * added error count of failed requests from gateway service to query service * added success metric in the gateway service * added success metric in gateway service * spotless apply fix for the github actions * adressing the PR comments 1 * spotlessApply fix * changed the metric tags * adressing the PR comments 2
2 participants
Description
Currently, there are no metrics to track the number of failed requests in the data query layer. Adding an error counter to track requests failing in gateway service. The metric is exposed on /metrics endpoint.
For query-service - https://github.com/hypertrace/gateway-service/pull/128