From ac58dc4dbbb6faf48ead2e3b312eab1fff257067 Mon Sep 17 00:00:00 2001 From: Jens Schulze Date: Mon, 3 Feb 2025 11:25:42 +0100 Subject: [PATCH 1/2] capture errors in telemetry middleware --- .../monitoring/datadog/DatadogMiddleware.java | 24 +++++++++++---- .../monitoring/datadog/DatadogUtils.java | 29 ++++++++++++++++--- .../datadog/statsd/DatadogMiddleware.java | 17 +++++++++-- .../newrelic/NewRelicTelemetryMiddleware.java | 25 ++++++++++++++-- .../OpenTelemetryMiddleware.java | 17 +++++++++-- 5 files changed, 93 insertions(+), 19 deletions(-) diff --git a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java index a7f8fe8c10b..6f54fd91a46 100644 --- a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java +++ b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java @@ -12,6 +12,7 @@ import com.datadog.api.client.ApiException; import com.datadog.api.client.v2.api.MetricsApi; +import io.vrap.rmf.base.client.ApiHttpException; import io.vrap.rmf.base.client.ApiHttpRequest; import io.vrap.rmf.base.client.ApiHttpResponse; import io.vrap.rmf.base.client.http.TelemetryMiddleware; @@ -51,14 +52,25 @@ public DatadogMiddleware(final MetricsApi apiInstance) { public CompletableFuture> invoke(ApiHttpRequest request, Function>> next) { final Instant start = Instant.now(); - return next.apply(request).thenApply(response -> { + return next.apply(request).handle((response, throwable) -> { + final int statusCode; + if (response != null) { + statusCode = response.getStatusCode(); + } + else { + if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { + statusCode = ((ApiHttpException) throwable).getResponse().getStatusCode(); + } + else { + statusCode = 0; + } + } try { submitClientDurationMetric(request, apiInstance, Duration.between(start, Instant.now()).toMillis(), - response); - submitTotalRequestsMetric(request, apiInstance, response); - - if (response.getStatusCode() >= 400) { - submitErrorRequestsMetric(request, apiInstance, response); + statusCode); + submitTotalRequestsMetric(request, apiInstance, statusCode); + if (statusCode >= 400 || throwable != null) { + submitErrorRequestsMetric(request, apiInstance, statusCode); } } catch (ApiException e) { diff --git a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogUtils.java b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogUtils.java index e645a3de86a..bdc2d105ce2 100644 --- a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogUtils.java +++ b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogUtils.java @@ -28,29 +28,50 @@ public class DatadogUtils { protected static void submitClientDurationMetric(final ApiHttpRequest request, final MetricsApi apiInstance, final double durationInMillis, final ApiHttpResponse response) throws ApiException { + submitClientDurationMetric(request, apiInstance, durationInMillis, response.getStatusCode()); + } + + protected static void submitClientDurationMetric(final ApiHttpRequest request, final MetricsApi apiInstance, + final double durationInMillis, final int statusCode) throws ApiException { final String name = PREFIX + "." + CLIENT_DURATION; final MetricIntakeType type = MetricIntakeType.UNSPECIFIED; - submitMetricWithHttpTags(name, durationInMillis, type, "ms", request, apiInstance, response); + submitMetricWithHttpTags(name, durationInMillis, type, "ms", request, apiInstance, statusCode); } protected static void submitErrorRequestsMetric(final ApiHttpRequest request, final MetricsApi apiInstance, final ApiHttpResponse response) throws ApiException { + submitErrorRequestsMetric(request, apiInstance, response.getStatusCode()); + } + + protected static void submitErrorRequestsMetric(final ApiHttpRequest request, final MetricsApi apiInstance, + final int statusCode) throws ApiException { final String name = PREFIX + "." + CLIENT_REQUEST_ERROR; final MetricIntakeType count = MetricIntakeType.COUNT; - submitMetricWithHttpTags(name, 1.0, count, "count", request, apiInstance, response); + submitMetricWithHttpTags(name, 1.0, count, "count", request, apiInstance, statusCode); } protected static void submitTotalRequestsMetric(final ApiHttpRequest request, final MetricsApi apiInstance, final ApiHttpResponse response) throws ApiException { + submitTotalRequestsMetric(request, apiInstance, response.getStatusCode()); + } + + protected static void submitTotalRequestsMetric(final ApiHttpRequest request, final MetricsApi apiInstance, + final int statusCode) throws ApiException { final String name = PREFIX + "." + CLIENT_REQUEST_TOTAL; final MetricIntakeType count = MetricIntakeType.COUNT; - submitMetricWithHttpTags(name, 1.0, count, "count", request, apiInstance, response); + submitMetricWithHttpTags(name, 1.0, count, "count", request, apiInstance, statusCode); } private static void submitMetricWithHttpTags(final String name, final double value, final MetricIntakeType type, final String unit, final ApiHttpRequest request, final MetricsApi apiInstance, final ApiHttpResponse response) throws ApiException { - final List tags = Arrays.asList(format("%s:%s", HTTP_RESPONSE_STATUS_CODE, response.getStatusCode()), + submitMetricWithHttpTags(name, value, type, unit, request, apiInstance, response.getStatusCode()); + } + + private static void submitMetricWithHttpTags(final String name, final double value, final MetricIntakeType type, + final String unit, final ApiHttpRequest request, final MetricsApi apiInstance, final int statusCode) + throws ApiException { + final List tags = Arrays.asList(format("%s:%s", HTTP_RESPONSE_STATUS_CODE, statusCode), format("%s:%s", HTTP_REQUEST_METHOD, request.getMethod().name()), format("%s:%s", SERVER_ADDRESS, request.getUri().getHost())); if (request.getUri().getPort() > 0) { diff --git a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/statsd/DatadogMiddleware.java b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/statsd/DatadogMiddleware.java index 8cecfdc2213..d6142f196a1 100644 --- a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/statsd/DatadogMiddleware.java +++ b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/statsd/DatadogMiddleware.java @@ -13,6 +13,7 @@ import com.timgroup.statsd.StatsDClient; +import io.vrap.rmf.base.client.ApiHttpException; import io.vrap.rmf.base.client.ApiHttpRequest; import io.vrap.rmf.base.client.ApiHttpResponse; import io.vrap.rmf.base.client.http.TelemetryMiddleware; @@ -49,9 +50,19 @@ public CompletableFuture> invoke(ApiHttpRequest request, Function>> next) { final Instant start = Instant.now(); - return next.apply(request).thenApply(response -> { + return next.apply(request).handle((response, throwable) -> { final List tags = new ArrayList<>(4); - tags.add(format("%s:%s", HTTP_RESPONSE_STATUS_CODE, response.getStatusCode())); + final int statusCode; + if (response != null) { + statusCode = response.getStatusCode(); + } + else if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { + statusCode = ((ApiHttpException) throwable).getResponse().getStatusCode(); + } + else { + statusCode = 0; + } + tags.add(format("%s:%s", HTTP_RESPONSE_STATUS_CODE, statusCode)); tags.add(format("%s:%s", HTTP_REQUEST_METHOD, request.getMethod().name())); tags.add(format("%s:%s", SERVER_ADDRESS, request.getUri().getHost())); if (request.getUri().getPort() > 0) { @@ -62,7 +73,7 @@ public CompletableFuture> invoke(ApiHttpRequest request, Duration.between(start, Instant.now()).toMillis(), tags.toArray(new String[0])); this.statsDClient.incrementCounter(PREFIX + "." + CLIENT_REQUEST_TOTAL, tags.toArray(new String[0])); - if (response.getStatusCode() >= 400) { + if (statusCode >= 400 || throwable != null) { this.statsDClient.incrementCounter(PREFIX + "." + CLIENT_REQUEST_ERROR, tags.toArray(new String[0])); } return response; diff --git a/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java b/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java index ae10f63b995..c52a793186c 100644 --- a/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java +++ b/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java @@ -11,6 +11,7 @@ import com.newrelic.api.agent.*; +import io.vrap.rmf.base.client.ApiHttpException; import io.vrap.rmf.base.client.ApiHttpRequest; import io.vrap.rmf.base.client.ApiHttpResponse; import io.vrap.rmf.base.client.ContextApiHttpClientImpl; @@ -61,13 +62,31 @@ public CompletableFuture> invoke(ApiHttpRequest request, Optional token = context.map(NewRelicContext::getTransaction).map(Transaction::getToken); Optional segment = context.map(c -> c.getTransaction() .startSegment("commercetools", request.getMethod() + " " + request.getUri().getPath())); - return next.apply(request).thenApply(response -> { + return next.apply(request).handle((response, throwable) -> { token.ifPresent(Token::linkAndExpire); + + final int statusCode; + final String message; + if (response != null) { + statusCode = response.getStatusCode(); + message = response.getMessage(); + } + else { + if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { + ApiHttpResponse errorResponse = ((ApiHttpException) throwable).getResponse(); + statusCode = errorResponse.getStatusCode(); + message = errorResponse.getMessage(); + } + else { + statusCode = 0; + message = throwable.getMessage(); + } + } segment.ifPresent(s -> s.reportAsExternal(HttpParameters.library("commercetools-sdk-java-v2") .uri(request.getUri()) .procedure(request.getMethod().name()) .noInboundHeaders() - .status(response.getStatusCode(), response.getMessage()) + .status(statusCode, message) .build())); segment.ifPresent(Segment::end); @@ -75,7 +94,7 @@ public CompletableFuture> invoke(ApiHttpRequest request, NewRelic.recordResponseTimeMetric(PREFIX + CLIENT_DURATION, Duration.between(start, Instant.now()).toMillis()); - if (response.getStatusCode() >= 400) { + if (statusCode >= 400 || throwable != null) { NewRelic.incrementCounter(PREFIX + CLIENT_REQUEST_ERROR); } return response; diff --git a/commercetools/commercetools-monitoring-opentelemetry/src/main/java/com/commercetools/monitoring/opentelemetry/OpenTelemetryMiddleware.java b/commercetools/commercetools-monitoring-opentelemetry/src/main/java/com/commercetools/monitoring/opentelemetry/OpenTelemetryMiddleware.java index 61ec108cb37..db3c929c3c4 100644 --- a/commercetools/commercetools-monitoring-opentelemetry/src/main/java/com/commercetools/monitoring/opentelemetry/OpenTelemetryMiddleware.java +++ b/commercetools/commercetools-monitoring-opentelemetry/src/main/java/com/commercetools/monitoring/opentelemetry/OpenTelemetryMiddleware.java @@ -13,6 +13,7 @@ import io.opentelemetry.api.metrics.LongCounter; import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; +import io.vrap.rmf.base.client.ApiHttpException; import io.vrap.rmf.base.client.ApiHttpRequest; import io.vrap.rmf.base.client.ApiHttpResponse; import io.vrap.rmf.base.client.http.TelemetryMiddleware; @@ -67,9 +68,19 @@ public OpenTelemetryMiddleware(final OpenTelemetry otel, final boolean enableHis public CompletableFuture> invoke(ApiHttpRequest request, Function>> next) { Instant start = Instant.now(); - return next.apply(request).thenApply(response -> { + return next.apply(request).handle((response, throwable) -> { + final int statusCode; + if (response != null) { + statusCode = response.getStatusCode(); + } + else if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { + statusCode = ((ApiHttpException) throwable).getResponse().getStatusCode(); + } + else { + statusCode = 0; + } AttributesBuilder builder = Attributes.builder() - .put(OpenTelemetryInfo.HTTP_RESPONSE_STATUS_CODE, response.getStatusCode()) + .put(OpenTelemetryInfo.HTTP_RESPONSE_STATUS_CODE, statusCode) .put(OpenTelemetryInfo.HTTP_REQUEST_METHOD, request.getMethod().name()) .put(OpenTelemetryInfo.SERVER_ADDRESS, request.getUri().getHost()); if (request.getUri().getPort() > 0) { @@ -79,7 +90,7 @@ public CompletableFuture> invoke(ApiHttpRequest request, Optional.ofNullable(histogram) .ifPresent(h -> h.record(Duration.between(start, Instant.now()).toMillis(), attributes)); requestCounter.add(1, attributes); - if (response.getStatusCode() >= 400) { + if (statusCode >= 400 || throwable != null) { errorCounter.add(1, attributes); } return response; From 79578ef6f23996863a94ac380b3efd922efe1beb Mon Sep 17 00:00:00 2001 From: Jens Schulze Date: Mon, 3 Feb 2025 11:29:05 +0100 Subject: [PATCH 2/2] reduce nesting in telemetry middlewares --- .../monitoring/datadog/DatadogMiddleware.java | 10 ++++------ .../newrelic/NewRelicTelemetryMiddleware.java | 17 ++++++++--------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java index 6f54fd91a46..a32bb2d6db3 100644 --- a/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java +++ b/commercetools/commercetools-monitoring-datadog/src/main/java/com/commercetools/monitoring/datadog/DatadogMiddleware.java @@ -57,13 +57,11 @@ public CompletableFuture> invoke(ApiHttpRequest request, if (response != null) { statusCode = response.getStatusCode(); } + else if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { + statusCode = ((ApiHttpException) throwable).getResponse().getStatusCode(); + } else { - if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { - statusCode = ((ApiHttpException) throwable).getResponse().getStatusCode(); - } - else { - statusCode = 0; - } + statusCode = 0; } try { submitClientDurationMetric(request, apiInstance, Duration.between(start, Instant.now()).toMillis(), diff --git a/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java b/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java index c52a793186c..7d08b01cba0 100644 --- a/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java +++ b/commercetools/commercetools-monitoring-newrelic/src/main/java/com/commercetools/monitoring/newrelic/NewRelicTelemetryMiddleware.java @@ -71,17 +71,16 @@ public CompletableFuture> invoke(ApiHttpRequest request, statusCode = response.getStatusCode(); message = response.getMessage(); } + else if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { + ApiHttpResponse errorResponse = ((ApiHttpException) throwable).getResponse(); + statusCode = errorResponse.getStatusCode(); + message = errorResponse.getMessage(); + } else { - if (throwable instanceof ApiHttpException && ((ApiHttpException) throwable).getResponse() != null) { - ApiHttpResponse errorResponse = ((ApiHttpException) throwable).getResponse(); - statusCode = errorResponse.getStatusCode(); - message = errorResponse.getMessage(); - } - else { - statusCode = 0; - message = throwable.getMessage(); - } + statusCode = 0; + message = throwable.getMessage(); } + segment.ifPresent(s -> s.reportAsExternal(HttpParameters.library("commercetools-sdk-java-v2") .uri(request.getUri()) .procedure(request.getMethod().name())