From 70821f4ba3d949d83fc0e72fbfd1db87d4e5976a Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Sun, 11 Jul 2021 03:01:36 +0530 Subject: [PATCH 1/5] Configurable timeouts for attribute and config service --- .../core/graphql/attributes/AttributeClient.java | 13 ++++++++----- .../service/DefaultGraphQlServiceConfig.java | 12 ++++++++++++ .../graphql/spi/config/GraphQlServiceConfig.java | 2 ++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java b/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java index 80e53a8f..affa30bb 100644 --- a/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java +++ b/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java @@ -18,10 +18,11 @@ @Singleton class AttributeClient { - private static final int DEFAULT_DEADLINE_SEC = 10; + private final AttributeServiceStub attributeServiceClient; private final GrpcContextBuilder grpcContextBuilder; private final AttributeModelTranslator translator; + private final GraphQlServiceConfig serviceConfig; @Inject AttributeClient( @@ -32,12 +33,13 @@ class AttributeClient { AttributeModelTranslator translator) { this.grpcContextBuilder = grpcContextBuilder; this.translator = translator; + this.serviceConfig = serviceConfig; this.attributeServiceClient = newStub( - grpcChannelRegistry.forAddress( - serviceConfig.getAttributeServiceHost(), - serviceConfig.getAttributeServicePort())) + grpcChannelRegistry.forAddress( + serviceConfig.getAttributeServiceHost(), + serviceConfig.getAttributeServicePort())) .withCallCredentials(credentials); } @@ -46,7 +48,8 @@ public Observable queryAll(GraphQlRequestContext requestContext) .stream( streamObserver -> this.attributeServiceClient - .withDeadlineAfter(DEFAULT_DEADLINE_SEC, TimeUnit.SECONDS) + .withDeadlineAfter(serviceConfig.getAttributeServiceTimeout().toMillis(), + TimeUnit.MILLISECONDS) .findAll(Empty.getDefaultInstance(), streamObserver)) .mapOptional(this.translator::translate); } diff --git a/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java b/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java index e3171a03..aeacea22 100644 --- a/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java +++ b/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java @@ -20,6 +20,7 @@ class DefaultGraphQlServiceConfig implements GraphQlServiceConfig { private static final String ATTRIBUTE_SERVICE_HOST_PROPERTY = "attribute.service.host"; private static final String ATTRIBUTE_SERVICE_PORT_PROPERTY = "attribute.service.port"; + private static final String ATTRIBUTE_SERVICE_CLIENT_TIMEOUT = "attribute.service.timeout"; private static final String GATEWAY_SERVICE_HOST_PROPERTY = "gateway.service.host"; private static final String GATEWAY_SERVICE_PORT_PROPERTY = "gateway.service.port"; @@ -33,6 +34,7 @@ class DefaultGraphQlServiceConfig implements GraphQlServiceConfig { private final int maxIoThreads; private final String attributeServiceHost; private final int attributeServicePort; + private final Duration attributeServiceTimeout; private final String gatewayServiceHost; private final int gatewayServicePort; private final Duration gatewayServiceTimeout; @@ -47,6 +49,11 @@ class DefaultGraphQlServiceConfig implements GraphQlServiceConfig { this.attributeServiceHost = untypedConfig.getString(ATTRIBUTE_SERVICE_HOST_PROPERTY); this.attributeServicePort = untypedConfig.getInt(ATTRIBUTE_SERVICE_PORT_PROPERTY); + // fallback timeout: 10s + this.attributeServiceTimeout = + untypedConfig.hasPath(ATTRIBUTE_SERVICE_CLIENT_TIMEOUT) + ? untypedConfig.getDuration(ATTRIBUTE_SERVICE_CLIENT_TIMEOUT) + : Duration.ofSeconds(10); this.gatewayServiceHost = untypedConfig.getString(GATEWAY_SERVICE_HOST_PROPERTY); this.gatewayServicePort = untypedConfig.getInt(GATEWAY_SERVICE_PORT_PROPERTY); // fallback timeout: 10s @@ -96,6 +103,11 @@ public int getAttributeServicePort() { return this.attributeServicePort; } + @Override + public Duration getAttributeServiceTimeout() { + return attributeServiceTimeout; + } + @Override public String getGatewayServiceHost() { return this.gatewayServiceHost; diff --git a/hypertrace-core-graphql-spi/src/main/java/org/hypertrace/core/graphql/spi/config/GraphQlServiceConfig.java b/hypertrace-core-graphql-spi/src/main/java/org/hypertrace/core/graphql/spi/config/GraphQlServiceConfig.java index 310666f7..f38a7aa3 100644 --- a/hypertrace-core-graphql-spi/src/main/java/org/hypertrace/core/graphql/spi/config/GraphQlServiceConfig.java +++ b/hypertrace-core-graphql-spi/src/main/java/org/hypertrace/core/graphql/spi/config/GraphQlServiceConfig.java @@ -21,6 +21,8 @@ public interface GraphQlServiceConfig { int getAttributeServicePort(); + Duration getAttributeServiceTimeout(); + String getGatewayServiceHost(); int getGatewayServicePort(); From df5fcb87eac7418961e5e96ecb3ce78d7e68c5ed Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Sun, 11 Jul 2021 11:44:39 +0530 Subject: [PATCH 2/5] Spotless apply --- .../core/graphql/attributes/AttributeClient.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java b/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java index affa30bb..38eb061a 100644 --- a/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java +++ b/hypertrace-core-graphql-attribute-store/src/main/java/org/hypertrace/core/graphql/attributes/AttributeClient.java @@ -37,9 +37,9 @@ class AttributeClient { this.attributeServiceClient = newStub( - grpcChannelRegistry.forAddress( - serviceConfig.getAttributeServiceHost(), - serviceConfig.getAttributeServicePort())) + grpcChannelRegistry.forAddress( + serviceConfig.getAttributeServiceHost(), + serviceConfig.getAttributeServicePort())) .withCallCredentials(credentials); } @@ -48,7 +48,8 @@ public Observable queryAll(GraphQlRequestContext requestContext) .stream( streamObserver -> this.attributeServiceClient - .withDeadlineAfter(serviceConfig.getAttributeServiceTimeout().toMillis(), + .withDeadlineAfter( + serviceConfig.getAttributeServiceTimeout().toMillis(), TimeUnit.MILLISECONDS) .findAll(Empty.getDefaultInstance(), streamObserver)) .mapOptional(this.translator::translate); From 97e1ed8715bd2eb346cc8a1beeae0221934916d3 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Sun, 11 Jul 2021 11:50:06 +0530 Subject: [PATCH 3/5] Refactored logic around default client timeout in case of missing config --- .../service/DefaultGraphQlServiceConfig.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java b/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java index aeacea22..6fc9bdf3 100644 --- a/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java +++ b/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java @@ -8,6 +8,8 @@ class DefaultGraphQlServiceConfig implements GraphQlServiceConfig { + private static final Long DEFAULT_CLIENT_TIMEOUT_SECONDS = 10L; + private static final String SERVICE_NAME_CONFIG = "service.name"; private static final String SERVICE_PORT_CONFIG = "service.port"; @@ -49,18 +51,13 @@ class DefaultGraphQlServiceConfig implements GraphQlServiceConfig { this.attributeServiceHost = untypedConfig.getString(ATTRIBUTE_SERVICE_HOST_PROPERTY); this.attributeServicePort = untypedConfig.getInt(ATTRIBUTE_SERVICE_PORT_PROPERTY); - // fallback timeout: 10s this.attributeServiceTimeout = - untypedConfig.hasPath(ATTRIBUTE_SERVICE_CLIENT_TIMEOUT) - ? untypedConfig.getDuration(ATTRIBUTE_SERVICE_CLIENT_TIMEOUT) - : Duration.ofSeconds(10); + getTimeoutOrFallback(() -> untypedConfig.getDuration(ATTRIBUTE_SERVICE_CLIENT_TIMEOUT)); + this.gatewayServiceHost = untypedConfig.getString(GATEWAY_SERVICE_HOST_PROPERTY); this.gatewayServicePort = untypedConfig.getInt(GATEWAY_SERVICE_PORT_PROPERTY); - // fallback timeout: 10s this.gatewayServiceTimeout = - untypedConfig.hasPath(GATEWAY_SERVICE_CLIENT_TIMEOUT) - ? untypedConfig.getDuration(GATEWAY_SERVICE_CLIENT_TIMEOUT) - : Duration.ofSeconds(10); + getTimeoutOrFallback(() -> untypedConfig.getDuration(GATEWAY_SERVICE_CLIENT_TIMEOUT)); } @Override @@ -123,6 +120,14 @@ public Duration getGatewayServiceTimeout() { return gatewayServiceTimeout; } + private Duration getTimeoutOrFallback(Supplier durationSupplier) { + try { + return durationSupplier.get(); + } catch (Throwable unused) { + return Duration.ofSeconds(DEFAULT_CLIENT_TIMEOUT_SECONDS); + } + } + private Optional optionallyGet(Supplier valueSupplier) { try { return Optional.ofNullable(valueSupplier.get()); From dbd1d8621a3bf96f007e613115f0a4d9b9e20c4a Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Mon, 12 Jul 2021 13:08:36 +0530 Subject: [PATCH 4/5] .synk update expiry date --- .snyk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.snyk b/.snyk index fbd83903..1ca9ad72 100644 --- a/.snyk +++ b/.snyk @@ -5,6 +5,6 @@ ignore: SNYK-JAVA-IONETTY-1042268: - '*': reason: No replacement available - expires: 2021-06-30T00:00:00.000Z + expires: 2021-09-01T00:00:00.000Z patch: {} From bc650209dc19bb9e9ccbe71b74ea1bb750071584 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Mon, 12 Jul 2021 17:19:40 +0530 Subject: [PATCH 5/5] Reuse optionallyGet in the implementation of getTimeoutOrFallback --- .../core/graphql/service/DefaultGraphQlServiceConfig.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java b/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java index 6fc9bdf3..538bd4f5 100644 --- a/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java +++ b/hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/DefaultGraphQlServiceConfig.java @@ -121,11 +121,8 @@ public Duration getGatewayServiceTimeout() { } private Duration getTimeoutOrFallback(Supplier durationSupplier) { - try { - return durationSupplier.get(); - } catch (Throwable unused) { - return Duration.ofSeconds(DEFAULT_CLIENT_TIMEOUT_SECONDS); - } + return optionallyGet(durationSupplier) + .orElse(Duration.ofSeconds(DEFAULT_CLIENT_TIMEOUT_SECONDS)); } private Optional optionallyGet(Supplier valueSupplier) {