Merge branch '1.8.x' into 1.9.x

Author: jonatan-ivanov

implementations/micrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v1/DynatraceExporterV1.java

@@ -53,8 +53,8 @@
  */
 public class DynatraceExporterV1 extends AbstractDynatraceExporter {
 
-    private static final int MAX_MESSAGE_SIZE = 15360; // max message size in bytes that
-                                                       // Dynatrace will accept
+    // max message size in bytes that Dynatrace will accept
+    private static final int MAX_MESSAGE_SIZE = 15360;
 
     private final InternalLogger logger = InternalLoggerFactory.getInstance(DynatraceExporterV1.class);
 
@@ -187,43 +187,73 @@ private boolean isCustomMetricCreated(final DynatraceTimeSeries timeSeries) {
 
     // VisibleForTesting
     void putCustomMetric(final DynatraceMetricDefinition customMetric) {
+        HttpSender.Request.Builder requestBuilder;
         try {
-            httpClient
+            requestBuilder = httpClient
                     .put(customMetricEndpointTemplate + customMetric.getMetricId() + "?api-token=" + config.apiToken())
-                    .withJsonContent(customMetric.asJson()).send().onSuccess(response -> {
-                        logger.debug("created {} as custom metric in Dynatrace", customMetric.getMetricId());
-                        createdCustomMetrics.add(customMetric.getMetricId());
-                    })
-                    .onError(response -> logger.error(
-                            "failed to create custom metric {} in Dynatrace: Error Code={}, Response Body={}",
-                            customMetric.getMetricId(), response.code(), response.body()));
+                    .withJsonContent(customMetric.asJson());
         }
-        catch (Throwable e) {
+        catch (Exception ex) {
             if (logger.isErrorEnabled()) {
-                logger.error("failed to create custom metric in Dynatrace: " + customMetric.getMetricId(), e);
+                logger.error("failed to build request: {}", redactToken(ex.getMessage()));
             }
+            return; // don't try to export data points, the request can't be built
+        }
+
+        HttpSender.Response httpResponse = trySendHttpRequest(requestBuilder);
+
+        if (httpResponse != null) {
+            httpResponse.onSuccess(response -> {
+                logger.debug("created {} as custom metric in Dynatrace", customMetric.getMetricId());
+                createdCustomMetrics.add(customMetric.getMetricId());
+            }).onError(response -> logger.error(
+                    "failed to create custom metric {} in Dynatrace: Error Code={}, Response Body={}",
+                    customMetric.getMetricId(), response.code(), response.body()));
         }
     }
 
     private void postCustomMetricValues(String type, String group, List<DynatraceTimeSeries> timeSeries,
             String customDeviceMetricEndpoint) {
-        try {
-            for (DynatraceBatchedPayload postMessage : createPostMessages(type, group, timeSeries)) {
-                httpClient.post(customDeviceMetricEndpoint).withJsonContent(postMessage.payload).send()
-                        .onSuccess(response -> {
-                            if (logger.isDebugEnabled()) {
-                                logger.debug("successfully sent {} metrics to Dynatrace ({} bytes).",
-                                        postMessage.metricCount, postMessage.payload.getBytes(UTF_8).length);
-                            }
-                        }).onError(response -> {
-                            logger.error("failed to send metrics to Dynatrace: Error Code={}, Response Body={}",
-                                    response.code(), response.body());
-                            logger.debug("failed metrics payload: {}", postMessage.payload);
-                        });
+        for (DynatraceBatchedPayload postMessage : createPostMessages(type, group, timeSeries)) {
+            HttpSender.Request.Builder requestBuilder;
+            try {
+                requestBuilder = httpClient.post(customDeviceMetricEndpoint).withJsonContent(postMessage.payload);
+            }
+            catch (Exception ex) {
+                if (logger.isErrorEnabled()) {
+                    logger.error("failed to build request: {}", redactToken(ex.getMessage()));
+                }
+
+                return; // don't try to export data points, the request can't be built
+            }
+
+            HttpSender.Response httpResponse = trySendHttpRequest(requestBuilder);
+
+            if (httpResponse != null) {
+                httpResponse.onSuccess(response -> {
+                    if (logger.isDebugEnabled()) {
+                        logger.debug("successfully sent {} metrics to Dynatrace ({} bytes).", postMessage.metricCount,
+                                postMessage.payload.getBytes(UTF_8).length);
+                    }
+                }).onError(response -> {
+                    logger.error("failed to send metrics to Dynatrace: Error Code={}, Response Body={}",
+                            response.code(), response.body());
+                    logger.debug("failed metrics payload: {}", postMessage.payload);
+                });
             }
         }
+    }
+
+    // VisibleForTesting
+    HttpSender.Response trySendHttpRequest(HttpSender.Request.Builder requestBuilder) {
+        try {
+            return requestBuilder.send();
+        }
         catch (Throwable e) {
-            logger.error("failed to send metrics to Dynatrace", e);
+            if (logger.isErrorEnabled()) {
+                logger.error("failed to send metrics to Dynatrace: {}", redactToken(e.getMessage()));
+            }
+            return null;
         }
     }
 
@@ -279,4 +309,8 @@ private Meter.Id idWithSuffix(Meter.Id id, String suffix) {
         return id.withName(id.getName() + "." + suffix);
     }
 
+    private String redactToken(String message) {
+        return message.replace(config.apiToken(), "<redacted>");
+    }
+
 }

implementations/micrometer-registry-dynatrace/src/test/java/io/micrometer/dynatrace/v1/DynatraceExporterV1Test.java

@@ -19,6 +19,7 @@
 import io.micrometer.core.instrument.*;
 import io.micrometer.core.instrument.config.validate.ValidationException;
 import io.micrometer.core.ipc.http.HttpSender;
+import io.micrometer.core.util.internal.logging.LogEvent;
 import io.micrometer.core.util.internal.logging.MockLogger;
 import io.micrometer.core.util.internal.logging.MockLoggerFactory;
 import io.micrometer.dynatrace.DynatraceApiVersion;
@@ -43,8 +44,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.isA;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 /**
  * Tests for {@link DynatraceExporterV1}.
@@ -266,16 +266,21 @@ void whenAllTsTooLargeEmptyMessageListReturned() {
 
     @Test
     void splitsWhenExactlyExceedingMaxByComma() {
+        // @formatter:off
         // comma needs to be considered when there is more than one time series
-        List<DynatraceBatchedPayload> messages = exporter.createPostMessages("my.type", "my.group",
+        List<DynatraceBatchedPayload> messages = exporter.createPostMessages(
+                "my.type",
+                "my.group",
                 // Max bytes: 15330 (excluding header/footer, 15360 with header/footer)
-                Arrays.asList(createTimeSeriesWithDimensions(750), // 14861 bytes
-                        createTimeSeriesWithDimensions(23, "asdfg"), // 469 bytes
-                                                                     // (overflows due to
-                                                                     // comma)
+                Arrays.asList(
+                        createTimeSeriesWithDimensions(750), // 14861 bytes
+                        // 469 bytes (overflows due to comma)
+                        createTimeSeriesWithDimensions(23, "asdfg"),
                         createTimeSeriesWithDimensions(750), // 14861 bytes
                         createTimeSeriesWithDimensions(22, "asd") // 468 bytes + comma
-                ));
+                )
+        );
+        // @formatter:on
         assertThat(messages).hasSize(3);
         assertThat(messages.get(0).metricCount).isEqualTo(1);
         assertThat(messages.get(1).metricCount).isEqualTo(1);
@@ -286,14 +291,19 @@ void splitsWhenExactlyExceedingMaxByComma() {
 
     @Test
     void countsPreviousAndNextComma() {
-        List<DynatraceBatchedPayload> messages = exporter.createPostMessages("my.type", null,
+        // @formatter:off
+        List<DynatraceBatchedPayload> messages = exporter.createPostMessages(
+                "my.type",
+                null,
                 // Max bytes: 15330 (excluding header/footer, 15360 with header/footer)
-                Arrays.asList(createTimeSeriesWithDimensions(750), // 14861 bytes
+                Arrays.asList(
+                        createTimeSeriesWithDimensions(750), // 14861 bytes
                         createTimeSeriesWithDimensions(10, "asdf"), // 234 bytes + comma
-                        createTimeSeriesWithDimensions(10, "asdf") // 234 bytes + comma =
-                                                                   // 15331 bytes
-                                                                   // (overflow)
-                ));
+                        // 234 bytes + comma = 15331 bytes (overflow)
+                        createTimeSeriesWithDimensions(10, "asdf")
+                )
+        );
+        // @formatter:on
         assertThat(messages).hasSize(2);
         assertThat(messages.get(0).metricCount).isEqualTo(2);
         assertThat(messages.get(1).metricCount).isEqualTo(1);
@@ -386,6 +396,128 @@ void failOnPostShouldHaveProperLogging() throws Throwable {
         assertThat(LOGGER.getLogEvents().get(1).getCause()).isNull();
     }
 
+    @Test
+    void testTokenShouldBeRedactedInPutFailure() {
+        HttpSender httpClient = spy(HttpSender.class);
+
+        String invalidUrl = "http://localhost###";
+        String apiToken = "this.is.a.fake.apiToken";
+
+        DynatraceExporterV1 exporter = getDynatraceExporterV1(httpClient, invalidUrl, apiToken);
+
+        meterRegistry.gauge("my.gauge", GAUGE_VALUE);
+        Gauge gauge = meterRegistry.find("my.gauge").gauge();
+
+        exporter.export(Collections.singletonList(gauge));
+
+        assertThat(LOGGER.getLogEvents())
+                // map to only keep the message strings
+                .extracting(LogEvent::getMessage)
+                .containsExactly(String.format(
+                        "failed to build request: Illegal character in fragment at index 17: %s/api/v1/timeseries/custom:my.gauge?api-token=<redacted>",
+                        invalidUrl));
+    }
+
+    @Test
+    void testTokenShouldBeRedactedInPostFailure() throws Throwable {
+        HttpSender httpClient = spy(HttpSender.class);
+
+        String invalidUrl = "http://localhost###";
+        String apiToken = "this.is.a.fake.apiToken";
+
+        HttpSender.Request.Builder builder = HttpSender.Request.build("http://localhost", httpClient);
+        // mock the PUT call, so we can even run the post call
+        doReturn(builder).when(httpClient).put(anyString());
+        doReturn(new HttpSender.Response(200, "")).when(httpClient).send(any(HttpSender.Request.class));
+
+        DynatraceExporterV1 exporter = getDynatraceExporterV1(httpClient, invalidUrl, apiToken);
+
+        meterRegistry.gauge("my.gauge", GAUGE_VALUE);
+        Gauge gauge = meterRegistry.find("my.gauge").gauge();
+
+        exporter.export(Collections.singletonList(gauge));
+
+        assertThat(LOGGER.getLogEvents())
+                // map to only keep the message strings
+                .extracting(LogEvent::getMessage).containsExactly(
+                        // the custom metric was created, meaning the PUT call succeeded
+                        "created custom:my.gauge as custom metric in Dynatrace",
+                        // the POST call throws an exception and the token is redacted
+                        String.format(
+                                "failed to build request: Illegal character in fragment at index 17: %s/api/v1/entity/infrastructure/custom/?api-token=<redacted>",
+                                invalidUrl));
+    }
+
+    @Test
+    void trySendHttpRequestSuccess() throws Throwable {
+        HttpSender httpClient = mock(HttpSender.class);
+        DynatraceExporterV1 exporter = FACTORY.injectLogger(() -> createExporter(httpClient));
+        HttpSender.Request.Builder reqBuilder = mock(HttpSender.Request.Builder.class);
+
+        // simulate a success response
+        when(reqBuilder.send()).thenReturn(new HttpSender.Response(200, ""));
+
+        // test that everything works and no error is logged
+        exporter.trySendHttpRequest(reqBuilder);
+        verify(reqBuilder).send();
+        assertThat(LOGGER.getLogEvents()).isEmpty();
+    }
+
+    @Test
+    void trySendHttpRequestErrorCode() throws Throwable {
+        HttpSender httpClient = mock(HttpSender.class);
+        DynatraceExporterV1 exporter = FACTORY.injectLogger(() -> createExporter(httpClient));
+        HttpSender.Request.Builder reqBuilder = mock(HttpSender.Request.Builder.class);
+
+        // simulate a failure response, errors are handled elsewhere
+        when(reqBuilder.send()).thenReturn(new HttpSender.Response(400, ""));
+
+        // test that everything works and no error is logged
+        exporter.trySendHttpRequest(reqBuilder);
+        verify(reqBuilder).send();
+        assertThat(LOGGER.getLogEvents()).isEmpty();
+    }
+
+    @Test
+    void trySendHttpRequestThrowsAndRedacts() throws Throwable {
+        HttpSender httpClient = mock(HttpSender.class);
+        String apiToken = "this.is.a.fake.apiToken";
+        DynatraceExporterV1 exporter = getDynatraceExporterV1(httpClient, "http://localhost", apiToken);
+
+        HttpSender.Request.Builder reqBuilder = mock(HttpSender.Request.Builder.class);
+
+        // Simulate that the request builder throws an exception.
+        // Should not happen if the endpoint is invalid, the URI is validated elsewhere.
+        String exceptionMessageTemplate = "Exception with the token: %s";
+        when(reqBuilder.send()).thenThrow(new Throwable(String.format(exceptionMessageTemplate, apiToken)));
+
+        exporter.trySendHttpRequest(reqBuilder);
+        verify(reqBuilder).send();
+        // assert that an error is logged
+        assertThat(LOGGER.getLogEvents()).hasSize(1).extracting(x -> x.getMessage()).containsExactlyInAnyOrder(
+                "failed to send metrics to Dynatrace: " + String.format(exceptionMessageTemplate, "<redacted>"));
+    }
+
+    private DynatraceExporterV1 getDynatraceExporterV1(HttpSender httpClient, String url, String apiToken) {
+        DynatraceExporterV1 exporter = FACTORY.injectLogger(() -> new DynatraceExporterV1(new DynatraceConfig() {
+            @Override
+            public String get(String key) {
+                return null;
+            }
+
+            @Override
+            public String apiToken() {
+                return apiToken;
+            }
+
+            @Override
+            public String uri() {
+                return url;
+            }
+        }, clock, httpClient));
+        return exporter;
+    }
+
     private DynatraceExporterV1 createExporter(HttpSender httpClient) {
         return new DynatraceExporterV1(config, clock, httpClient);
     }