This repository was archived by the owner on Jun 26, 2024. It is now read-only.
added error count of failed requests from gateway service to query service #128
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,21 @@ | ||
| package org.hypertrace.gateway.service; | ||
|
|
||
| import com.google.common.base.Preconditions; | ||
| import com.google.common.collect.ImmutableMap; | ||
| import com.google.protobuf.ServiceException; | ||
| import com.typesafe.config.Config; | ||
| import io.grpc.ManagedChannel; | ||
| import io.grpc.ManagedChannelBuilder; | ||
| import io.grpc.stub.StreamObserver; | ||
| import io.micrometer.core.instrument.Counter; | ||
| import java.util.Optional; | ||
| import java.util.concurrent.ExecutorService; | ||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.hypertrace.core.attribute.service.client.AttributeServiceClient; | ||
| import org.hypertrace.core.attribute.service.client.config.AttributeServiceClientConfig; | ||
| import org.hypertrace.core.query.service.client.QueryServiceClient; | ||
| import org.hypertrace.core.query.service.client.QueryServiceConfig; | ||
| import org.hypertrace.core.serviceframework.metrics.PlatformMetricsRegistry; | ||
| import org.hypertrace.entity.query.service.client.EntityQueryServiceClient; | ||
| import org.hypertrace.entity.service.client.config.EntityServiceClientConfig; | ||
| import org.hypertrace.gateway.service.baseline.BaselineService; | ||
|
|
@@ -61,6 +64,11 @@ public class GatewayServiceImpl extends GatewayServiceGrpc.GatewayServiceImplBas | |
| private static final String REQUEST_TIMEOUT_CONFIG_KEY = "request.timeout"; | ||
| private static final int DEFAULT_REQUEST_TIMEOUT_MILLIS = 10000; | ||
|
|
||
| private Counter serviceResponseErrorCounter; | ||
| private Counter serviceResponseSuccessCounter; | ||
| private static final String ERROR_COUNTER_NAME = "hypertrace.gateway.response.errors"; | ||
|
Contributor
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. Any suggestions hypertrace_gateway_requests_status {"error"="true"} 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 : And in PR - hypertrace/query-service#138,
Contributor
Author
There was a problem hiding this comment.
Yes makes sense. Below looks good:
Yes my bad, keeping the same naming convention across both the services like - Pushing the above changes. |
||
| private static final String SUCCESS_COUNTER_NAME = "hypertrace.gateway.response.success"; | ||
|
|
||
| private final TracesService traceService; | ||
| private final SpanService spanService; | ||
| private final EntityService entityService; | ||
|
|
@@ -134,6 +142,14 @@ public GatewayServiceImpl(Config appConfig) { | |
| entityIdColumnsConfigs); | ||
| this.logEventsService = | ||
| new LogEventsService(queryServiceClient, qsRequestTimeout, attributeMetadataProvider); | ||
| initMetrics(); | ||
| } | ||
|
|
||
| private void initMetrics() { | ||
| serviceResponseErrorCounter = | ||
| PlatformMetricsRegistry.registerCounter(ERROR_COUNTER_NAME, ImmutableMap.of()); | ||
|
Contributor
There was a problem hiding this comment. If we go with the above metrics - #128 (comment), you may have to register a counter with And,
Contributor
Author
There was a problem hiding this comment. Yes. Making the change. |
||
| serviceResponseSuccessCounter = | ||
| PlatformMetricsRegistry.registerCounter(SUCCESS_COUNTER_NAME, ImmutableMap.of()); | ||
| } | ||
|
|
||
| private static int getRequestTimeoutMillis(Config config) { | ||
|
|
@@ -152,6 +168,7 @@ public void getTraces( | |
| Optional<String> tenantId = | ||
| org.hypertrace.core.grpcutils.context.RequestContext.CURRENT.get().getTenantId(); | ||
| if (tenantId.isEmpty()) { | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(new ServiceException("Tenant id is missing in the request.")); | ||
| return; | ||
| } | ||
|
|
@@ -167,8 +184,10 @@ public void getTraces( | |
| TracesResponse response = traceService.getTracesByFilter(requestContext, request); | ||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling traces request: {}", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
@@ -181,6 +200,7 @@ public void getSpans( | |
| Optional<String> tenantId = | ||
| org.hypertrace.core.grpcutils.context.RequestContext.CURRENT.get().getTenantId(); | ||
| if (tenantId.isEmpty()) { | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(new ServiceException("Tenant id is missing in the request.")); | ||
| return; | ||
| } | ||
|
|
@@ -195,8 +215,10 @@ public void getSpans( | |
| SpansResponse response = spanService.getSpansByFilter(context, request); | ||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling spans request: {}", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
@@ -211,6 +233,7 @@ public void getEntities( | |
| Optional<String> tenantId = | ||
| org.hypertrace.core.grpcutils.context.RequestContext.CURRENT.get().getTenantId(); | ||
| if (tenantId.isEmpty()) { | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(new ServiceException("Tenant id is missing in the request.")); | ||
| return; | ||
| } | ||
|
|
@@ -241,8 +264,10 @@ public void getEntities( | |
|
|
||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling entities request: {}.", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
@@ -257,6 +282,7 @@ public void updateEntity( | |
| Optional<String> tenantId = | ||
| org.hypertrace.core.grpcutils.context.RequestContext.CURRENT.get().getTenantId(); | ||
| if (tenantId.isEmpty()) { | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(new ServiceException("Tenant id is missing in the request.")); | ||
| return; | ||
| } | ||
|
|
@@ -275,8 +301,10 @@ public void updateEntity( | |
| } | ||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling UpdateEntityRequest: {}.", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
@@ -305,8 +333,10 @@ public void bulkUpdateEntities( | |
| LOG.debug("Received response: {}", response); | ||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling bulkUpdateEntities: {}.", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
@@ -317,6 +347,7 @@ public void getBaselineForEntities( | |
| Optional<String> tenantId = | ||
| org.hypertrace.core.grpcutils.context.RequestContext.CURRENT.get().getTenantId(); | ||
| if (tenantId.isEmpty()) { | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(new ServiceException("Tenant id is missing in the request.")); | ||
| return; | ||
| } | ||
|
|
@@ -334,8 +365,10 @@ public void getBaselineForEntities( | |
|
|
||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling entities request: {}.", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
@@ -345,6 +378,7 @@ public void explore(ExploreRequest request, StreamObserver<ExploreResponse> resp | |
| Optional<String> tenantId = | ||
| org.hypertrace.core.grpcutils.context.RequestContext.CURRENT.get().getTenantId(); | ||
| if (tenantId.isEmpty()) { | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(new ServiceException("Tenant id is missing in the request.")); | ||
| return; | ||
| } | ||
|
|
@@ -359,8 +393,10 @@ public void explore(ExploreRequest request, StreamObserver<ExploreResponse> resp | |
| .getRequestHeaders()); | ||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling explore request: {}", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
@@ -371,6 +407,7 @@ public void getLogEvents( | |
| Optional<String> tenantId = | ||
| org.hypertrace.core.grpcutils.context.RequestContext.CURRENT.get().getTenantId(); | ||
| if (tenantId.isEmpty()) { | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(new ServiceException("Tenant id is missing in the request.")); | ||
| return; | ||
| } | ||
|
|
@@ -385,8 +422,10 @@ public void getLogEvents( | |
| LogEventsResponse response = logEventsService.getLogEventsByFilter(context, request); | ||
| responseObserver.onNext(response); | ||
| responseObserver.onCompleted(); | ||
| serviceResponseSuccessCounter.increment(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error while handling logEvents request: {}", request, e); | ||
| serviceResponseErrorCounter.increment(); | ||
| responseObserver.onError(e); | ||
| } | ||
| } | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
serviceResponseErrorCounter->requestStatusErrorCounterserviceResponseSuccessCounter->requestStatusSuccessCounterThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.