From 746eef7768f2e78a4eab45cba7f81e4c2237185c Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Wed, 12 Oct 2022 01:17:46 -0400 Subject: [PATCH 1/5] change setDimensions to putDimensions for context copy --- .../cloudwatchlogs/emf/model/MetricDirective.java | 2 +- .../emf/logger/MetricsLoggerTest.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java b/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java index c4d166d1..0846b2c4 100644 --- a/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java +++ b/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java @@ -168,7 +168,7 @@ MetricDirective copyWithoutMetrics(boolean preserveDimensions) { metricDirective.shouldUseDefaultDimension = this.shouldUseDefaultDimension; if (preserveDimensions) { - metricDirective.setDimensions(this.dimensions); + this.dimensions.forEach(metricDirective::putDimensionSet); } return metricDirective; diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java index 4b163189..19f5a4e1 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java @@ -275,6 +275,19 @@ void flush_doesNotPreserveDimensions() expectDimension("Name", null); } + @Test + void whenMultipleFlush_withNoDimensionsChanges_keepDefaultDimensions() + throws InvalidMetricException, DimensionSetExceededException { + logger.putMetric("Count", 1); + + logger.flush(); + logger.flush(); + + expectDimension("LogGroup", "test-log-group"); + expectDimension("ServiceName", "test-env-name"); + expectDimension("ServiceType", "test-env-type"); + } + @Test void setDimensions_clearsAllDimensions() throws DimensionSetExceededException { MetricsLogger logger = new MetricsLogger(envProvider); From c9ff702dd98a5c24a239045447f49391ada22421 Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Wed, 12 Oct 2022 01:19:13 -0400 Subject: [PATCH 2/5] fixes for java target version downgrade --- examples/agent/src/main/java/agent/App.java | 6 +++++- examples/ecs-firelens/src/main/java/App.java | 5 ++++- .../emf/environment/AgentBasedEnvironment.java | 18 +++++++++++------- .../emf/environment/DefaultEnvironment.java | 12 ++++++++---- .../emf/logger/MetricsLoggerTest.java | 14 +++++++++----- .../emf/model/MetricDefinitionTest.java | 7 ++++--- .../emf/model/MetricsContextTest.java | 9 ++++++--- .../cloudwatchlogs/emf/model/RootNodeTest.java | 4 +++- .../emf/sinks/AgentSinkTest.java | 3 ++- .../emf/sinks/MultiSinkTest.java | 4 ++-- 10 files changed, 54 insertions(+), 28 deletions(-) diff --git a/examples/agent/src/main/java/agent/App.java b/examples/agent/src/main/java/agent/App.java index 9e17b133..163200be 100644 --- a/examples/agent/src/main/java/agent/App.java +++ b/examples/agent/src/main/java/agent/App.java @@ -23,7 +23,11 @@ public static void main(String[] args) { } catch (InvalidMetricException | InvalidDimensionException | DimensionSetExceededException e) { System.out.println(e); } - environment.getSink().shutdown().orTimeout(360_000L, TimeUnit.MILLISECONDS); + + try { + environment.getSink().shutdown().get(360_000L, TimeUnit.MILLISECONDS); + } catch (Exception ignored) { + } } private static void emitMetric(Environment environment) diff --git a/examples/ecs-firelens/src/main/java/App.java b/examples/ecs-firelens/src/main/java/App.java index 4f191bef..344aa003 100644 --- a/examples/ecs-firelens/src/main/java/App.java +++ b/examples/ecs-firelens/src/main/java/App.java @@ -51,7 +51,10 @@ public static void main(String[] args) throws Exception { private static void registerShutdownHook() { // https://aws.amazon.com/blogs/containers/graceful-shutdowns-with-ecs/ Signal.handle(new Signal("TERM"), sig -> { - env.getSink().shutdown().orTimeout(1_000L, TimeUnit.MILLISECONDS); + try { + env.getSink().shutdown().get(1_000L, TimeUnit.MILLISECONDS); + } catch (Exception ignored) { + } System.exit(0); }); } diff --git a/src/main/java/software/amazon/cloudwatchlogs/emf/environment/AgentBasedEnvironment.java b/src/main/java/software/amazon/cloudwatchlogs/emf/environment/AgentBasedEnvironment.java index 3f2cee07..232f54fa 100644 --- a/src/main/java/software/amazon/cloudwatchlogs/emf/environment/AgentBasedEnvironment.java +++ b/src/main/java/software/amazon/cloudwatchlogs/emf/environment/AgentBasedEnvironment.java @@ -16,6 +16,7 @@ package software.amazon.cloudwatchlogs.emf.environment; +import java.util.Optional; import lombok.extern.slf4j.Slf4j; import software.amazon.cloudwatchlogs.emf.Constants; import software.amazon.cloudwatchlogs.emf.config.Configuration; @@ -36,11 +37,14 @@ protected AgentBasedEnvironment(Configuration config) { @Override public String getName() { - if (config.getServiceName().isEmpty()) { - log.warn("Unknown ServiceName."); - return Constants.UNKNOWN; + Optional serviceName = config.getServiceName(); + + if (serviceName.isPresent()) { + return serviceName.get(); } - return config.getServiceName().get(); + + log.warn("Unknown ServiceName."); + return Constants.UNKNOWN; } @Override @@ -64,13 +68,13 @@ public String getLogStreamName() { public ISink getSink() { if (sink == null) { Endpoint endpoint; - if (config.getAgentEndpoint().isEmpty()) { + if (config.getAgentEndpoint().isPresent()) { + endpoint = Endpoint.fromURL(config.getAgentEndpoint().get()); + } else { log.info( "Endpoint is not defined. Using default: {}", Endpoint.DEFAULT_TCP_ENDPOINT); endpoint = Endpoint.DEFAULT_TCP_ENDPOINT; - } else { - endpoint = Endpoint.fromURL(config.getAgentEndpoint().get()); } sink = new AgentSink( diff --git a/src/main/java/software/amazon/cloudwatchlogs/emf/environment/DefaultEnvironment.java b/src/main/java/software/amazon/cloudwatchlogs/emf/environment/DefaultEnvironment.java index 90d20400..96c0e768 100644 --- a/src/main/java/software/amazon/cloudwatchlogs/emf/environment/DefaultEnvironment.java +++ b/src/main/java/software/amazon/cloudwatchlogs/emf/environment/DefaultEnvironment.java @@ -16,6 +16,7 @@ package software.amazon.cloudwatchlogs.emf.environment; +import java.util.Optional; import lombok.extern.slf4j.Slf4j; import software.amazon.cloudwatchlogs.emf.Constants; import software.amazon.cloudwatchlogs.emf.config.Configuration; @@ -37,11 +38,14 @@ public boolean probe() { @Override public String getType() { - if (config.getServiceType().isEmpty()) { - log.info("Unknown ServiceType"); - return Constants.UNKNOWN; + Optional serviceType = config.getServiceType(); + + if (serviceType.isPresent()) { + return serviceType.get(); } - return config.getServiceType().get(); + + log.info("Unknown ServiceType"); + return Constants.UNKNOWN; } @Override diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java index 19f5a4e1..7beaaaae 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java @@ -126,7 +126,8 @@ void whenSetDimension_withInvalidValue_thenThrowInvalidDimensionException( @Test void whenSetDimension_withNameTooLong_thenThrowDimensionException() { - String dimensionName = "a".repeat(Constants.MAX_DIMENSION_NAME_LENGTH + 1); + String dimensionName = + new String(new char[Constants.MAX_DIMENSION_NAME_LENGTH + 1]).replace("\0", "a"); String dimensionValue = "dimValue"; assertThrows( InvalidDimensionException.class, @@ -136,7 +137,8 @@ void whenSetDimension_withNameTooLong_thenThrowDimensionException() { @Test void whenSetDimension_withValueTooLong_thenThrowDimensionException() { String dimensionName = "dim"; - String dimensionValue = "a".repeat(Constants.MAX_DIMENSION_VALUE_LENGTH + 1); + String dimensionValue = + new String(new char[Constants.MAX_DIMENSION_VALUE_LENGTH + 1]).replace("\0", "a"); assertThrows( InvalidDimensionException.class, () -> DimensionSet.of(dimensionName, dimensionValue)); @@ -315,7 +317,8 @@ void whenSetDimensions_withMultipleFlush_thenClearsDimensions() @Test void whenPutMetric_withTooLongName_thenThrowInvalidMetricException() { - String name1 = "a".repeat(Constants.MAX_METRIC_NAME_LENGTH + 1); + String name1 = + new String(new char[Constants.MAX_METRIC_NAME_LENGTH + 1]).replace("\0", "a"); assertThrows(InvalidMetricException.class, () -> logger.putMetric(name1, 1)); } @@ -362,7 +365,8 @@ void whenSetNamespace_withInvalidValue_thenThrowInvalidNamespaceException(String @Test void whenSetNamespace_withNameTooLong_thenThrowInvalidNamespaceException() { - String namespace = "a".repeat(Constants.MAX_NAMESPACE_LENGTH + 1); + String namespace = + new String(new char[Constants.MAX_NAMESPACE_LENGTH + 1]).replace("\0", "a"); assertThrows(InvalidNamespaceException.class, () -> logger.setNamespace(namespace)); } @@ -479,7 +483,7 @@ void testUseDefaultEnvironmentOnResolverException() throws DimensionSetExceededE @Test void flush_doesNotPreserveMetrics() throws InvalidMetricException, InvalidDimensionException, - DimensionSetExceededException { + DimensionSetExceededException { MetricsLogger logger = new MetricsLogger(envProvider); logger.setDimensions(DimensionSet.of("Name", "Test")); logger.putMetric("Count", 1.0); diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionTest.java index 496262ae..cc1aeaff 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionTest.java @@ -20,7 +20,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import java.util.List; +import java.util.Arrays; +import java.util.Collections; import org.junit.Test; public class MetricDefinitionTest { @@ -51,9 +52,9 @@ public void testSerializeMetricDefinition() throws JsonProcessingException { @Test public void testAddValue() { MetricDefinition md = new MetricDefinition("Time", Unit.MICROSECONDS, 10); - assertEquals(List.of(10d), md.getValues()); + assertEquals(Collections.singletonList(10d), md.getValues()); md.addValue(20); - assertEquals(List.of(10d, 20d), md.getValues()); + assertEquals(Arrays.asList(10d, 20d), md.getValues()); } } diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricsContextTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricsContextTest.java index 78de1165..3a39baaa 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricsContextTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricsContextTest.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.databind.json.JsonMapper; import java.time.Instant; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import org.junit.jupiter.api.Assertions; @@ -101,7 +102,7 @@ void testSerializeAMetricWith101DataPoints() expectedValues.add((double) i); } Assertions.assertEquals(expectedValues, allMetrics.get(0).getValues()); - Assertions.assertEquals(List.of(100.0), allMetrics.get(1).getValues()); + Assertions.assertEquals(Collections.singletonList(100.0), allMetrics.get(1).getValues()); } @Test @@ -128,10 +129,12 @@ void testSerializeMetricsWith101DataPoints() expectedValues.add((double) i); } Assertions.assertEquals(expectedValues, metricsFromEvent1.get(0).getValues()); - Assertions.assertEquals(List.of(2.0), metricsFromEvent1.get(1).getValues()); + Assertions.assertEquals( + Collections.singletonList(2.0), metricsFromEvent1.get(1).getValues()); Assertions.assertEquals(1, metricsFromEvent2.size()); - Assertions.assertEquals(List.of(100.0), metricsFromEvent2.get(0).getValues()); + Assertions.assertEquals( + Collections.singletonList(100.0), metricsFromEvent2.get(0).getValues()); } @Test diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/model/RootNodeTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/model/RootNodeTest.java index 564e482e..1ca432dc 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/model/RootNodeTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/model/RootNodeTest.java @@ -19,6 +19,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.Arrays; import java.util.List; import java.util.Map; import org.junit.jupiter.api.Assertions; @@ -72,7 +73,8 @@ void testGetTargetMembers() mc.putProperty("Prop1", "PropValue1"); - Assertions.assertEquals(List.of(10.0, 20.0), rootNode.getTargetMembers().get("Count")); + Assertions.assertEquals( + Arrays.asList(10.0, 20.0), rootNode.getTargetMembers().get("Count")); Assertions.assertEquals(100.0, rootNode.getTargetMembers().get("Latency")); Assertions.assertEquals("DimVal1", rootNode.getTargetMembers().get("Dim1")); Assertions.assertEquals("PropValue1", rootNode.getTargetMembers().get("Prop1")); diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/AgentSinkTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/AgentSinkTest.java index 47f26835..42c32bea 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/AgentSinkTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/AgentSinkTest.java @@ -103,7 +103,8 @@ public void testEmptyLogGroupName() throws JsonProcessingException, InvalidMetri ObjectMapper objectMapper = new ObjectMapper(); Map emf_map = objectMapper.readValue( - fixture.client.getMessages().get(0), new TypeReference<>() {}); + fixture.client.getMessages().get(0), + new TypeReference>() {}); Map metadata = (Map) emf_map.get("_aws"); assertFalse(metadata.containsKey("LogGroupName")); diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/MultiSinkTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/MultiSinkTest.java index 103fd346..cb27a533 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/MultiSinkTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/sinks/MultiSinkTest.java @@ -28,8 +28,8 @@ public void shutdownClosesAllComponentSinks() { @Test public void shutdownCompletesExceptionallyIfComponentSinkCompletesExceptionally() { // arrange - CompletableFuture failedResult = - CompletableFuture.failedFuture(new RuntimeException()); + CompletableFuture failedResult = new CompletableFuture<>(); + failedResult.completeExceptionally(new RuntimeException()); TestSink sink1 = new TestSink(); TestSink sink2 = new TestSink(failedResult); MultiSink multiSink = MultiSink.builder().sink(sink1).sink(sink2).build(); From 11a386fd42f1b2295d71e840ae07ab0b222f74c9 Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Wed, 12 Oct 2022 01:20:04 -0400 Subject: [PATCH 3/5] fix: test not asserting anything --- .../cloudwatchlogs/emf/logger/MetricsLoggerTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java index 7beaaaae..27edd4dd 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java @@ -93,8 +93,8 @@ void whenDefaultDimension_DimensionValue_Empty() throws DimensionSetExceededExce assertEquals(1, sink.getContext().getDimensions().size()); Set dimensionKeys = sink.getContext().getDimensions().get(0).getDimensionKeys(); assertEquals(2, dimensionKeys.size()); - dimensionKeys.contains("ServiceName"); - dimensionKeys.contains("ServiceType"); + assertTrue(dimensionKeys.contains("ServiceName")); + assertTrue(dimensionKeys.contains("ServiceType")); } @Test @@ -105,8 +105,8 @@ void whenDefaultDimension_DimensionValue_Blank() throws DimensionSetExceededExce assertEquals(1, sink.getContext().getDimensions().size()); Set dimensionKeys = sink.getContext().getDimensions().get(0).getDimensionKeys(); assertEquals(2, dimensionKeys.size()); - dimensionKeys.contains("ServiceName"); - dimensionKeys.contains("ServiceType"); + assertTrue(dimensionKeys.contains("ServiceName")); + assertTrue(dimensionKeys.contains("ServiceType")); } @ParameterizedTest From 92136795a5064e17edb2e2851ea99059b13caaf1 Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Wed, 12 Oct 2022 01:27:04 -0400 Subject: [PATCH 4/5] spotless fix --- .../amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java index 27edd4dd..42f07685 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java @@ -483,7 +483,7 @@ void testUseDefaultEnvironmentOnResolverException() throws DimensionSetExceededE @Test void flush_doesNotPreserveMetrics() throws InvalidMetricException, InvalidDimensionException, - DimensionSetExceededException { + DimensionSetExceededException { MetricsLogger logger = new MetricsLogger(envProvider); logger.setDimensions(DimensionSet.of("Name", "Test")); logger.putMetric("Count", 1.0); From a7d16e979c5154592a9b9fdaa4ec52767048ee68 Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Wed, 12 Oct 2022 22:20:02 -0400 Subject: [PATCH 5/5] use arrays.fill for initialized char[] --- .../emf/logger/MetricsLoggerTest.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java index 42f07685..0539ccc1 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java @@ -20,6 +20,7 @@ import static org.mockito.Mockito.*; import java.time.Instant; +import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.concurrent.CompletableFuture; @@ -126,9 +127,11 @@ void whenSetDimension_withInvalidValue_thenThrowInvalidDimensionException( @Test void whenSetDimension_withNameTooLong_thenThrowDimensionException() { - String dimensionName = - new String(new char[Constants.MAX_DIMENSION_NAME_LENGTH + 1]).replace("\0", "a"); + char[] longName = new char[Constants.MAX_DIMENSION_NAME_LENGTH + 1]; + Arrays.fill(longName, 'a'); + String dimensionName = String.valueOf(longName); String dimensionValue = "dimValue"; + assertThrows( InvalidDimensionException.class, () -> DimensionSet.of(dimensionName, dimensionValue)); @@ -137,8 +140,10 @@ void whenSetDimension_withNameTooLong_thenThrowDimensionException() { @Test void whenSetDimension_withValueTooLong_thenThrowDimensionException() { String dimensionName = "dim"; - String dimensionValue = - new String(new char[Constants.MAX_DIMENSION_VALUE_LENGTH + 1]).replace("\0", "a"); + char[] longName = new char[Constants.MAX_DIMENSION_VALUE_LENGTH + 1]; + Arrays.fill(longName, 'a'); + String dimensionValue = String.valueOf(longName); + assertThrows( InvalidDimensionException.class, () -> DimensionSet.of(dimensionName, dimensionValue)); @@ -317,8 +322,10 @@ void whenSetDimensions_withMultipleFlush_thenClearsDimensions() @Test void whenPutMetric_withTooLongName_thenThrowInvalidMetricException() { - String name1 = - new String(new char[Constants.MAX_METRIC_NAME_LENGTH + 1]).replace("\0", "a"); + char[] longName = new char[Constants.MAX_METRIC_NAME_LENGTH + 1]; + Arrays.fill(longName, 'a'); + String name1 = new String(longName); + assertThrows(InvalidMetricException.class, () -> logger.putMetric(name1, 1)); } @@ -365,8 +372,10 @@ void whenSetNamespace_withInvalidValue_thenThrowInvalidNamespaceException(String @Test void whenSetNamespace_withNameTooLong_thenThrowInvalidNamespaceException() { - String namespace = - new String(new char[Constants.MAX_NAMESPACE_LENGTH + 1]).replace("\0", "a"); + char[] longName = new char[Constants.MAX_NAMESPACE_LENGTH + 1]; + Arrays.fill(longName, 'a'); + String namespace = new String(longName); + assertThrows(InvalidNamespaceException.class, () -> logger.setNamespace(namespace)); }