Skip to content

Commit be29384

Browse files
author
Mark Kuhn
authored
Fix: Context copying is disabling default dimensions (#128)
* change setDimensions to putDimensions for context copy * fixes for java target version downgrade
1 parent ac3e84c commit be29384

File tree

11 files changed

+80
-32
lines changed

11 files changed

+80
-32
lines changed

‎examples/agent/src/main/java/agent/App.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ public static void main(String[] args) {
2323
} catch (InvalidMetricException | InvalidDimensionException | DimensionSetExceededException e) {
2424
System.out.println(e);
2525
}
26-
environment.getSink().shutdown().orTimeout(360_000L, TimeUnit.MILLISECONDS);
26+
27+
try {
28+
environment.getSink().shutdown().get(360_000L, TimeUnit.MILLISECONDS);
29+
} catch (Exception ignored) {
30+
}
2731
}
2832

2933
private static void emitMetric(Environment environment)

‎examples/ecs-firelens/src/main/java/App.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ public static void main(String[] args) throws Exception {
5151
private static void registerShutdownHook() {
5252
// https://aws.amazon.com/blogs/containers/graceful-shutdowns-with-ecs/
5353
Signal.handle(new Signal("TERM"), sig -> {
54-
env.getSink().shutdown().orTimeout(1_000L, TimeUnit.MILLISECONDS);
54+
try {
55+
env.getSink().shutdown().get(1_000L, TimeUnit.MILLISECONDS);
56+
} catch (Exception ignored) {
57+
}
5558
System.exit(0);
5659
});
5760
}

‎src/main/java/software/amazon/cloudwatchlogs/emf/environment/AgentBasedEnvironment.java

+11-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package software.amazon.cloudwatchlogs.emf.environment;
1818

19+
import java.util.Optional;
1920
import lombok.extern.slf4j.Slf4j;
2021
import software.amazon.cloudwatchlogs.emf.Constants;
2122
import software.amazon.cloudwatchlogs.emf.config.Configuration;
@@ -36,11 +37,14 @@ protected AgentBasedEnvironment(Configuration config) {
3637

3738
@Override
3839
public String getName() {
39-
if (config.getServiceName().isEmpty()) {
40-
log.warn("Unknown ServiceName.");
41-
return Constants.UNKNOWN;
40+
Optional<String> serviceName = config.getServiceName();
41+
42+
if (serviceName.isPresent()) {
43+
return serviceName.get();
4244
}
43-
return config.getServiceName().get();
45+
46+
log.warn("Unknown ServiceName.");
47+
return Constants.UNKNOWN;
4448
}
4549

4650
@Override
@@ -64,13 +68,13 @@ public String getLogStreamName() {
6468
public ISink getSink() {
6569
if (sink == null) {
6670
Endpoint endpoint;
67-
if (config.getAgentEndpoint().isEmpty()) {
71+
if (config.getAgentEndpoint().isPresent()) {
72+
endpoint = Endpoint.fromURL(config.getAgentEndpoint().get());
73+
} else {
6874
log.info(
6975
"Endpoint is not defined. Using default: {}",
7076
Endpoint.DEFAULT_TCP_ENDPOINT);
7177
endpoint = Endpoint.DEFAULT_TCP_ENDPOINT;
72-
} else {
73-
endpoint = Endpoint.fromURL(config.getAgentEndpoint().get());
7478
}
7579
sink =
7680
new AgentSink(

‎src/main/java/software/amazon/cloudwatchlogs/emf/environment/DefaultEnvironment.java

+8-4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package software.amazon.cloudwatchlogs.emf.environment;
1818

19+
import java.util.Optional;
1920
import lombok.extern.slf4j.Slf4j;
2021
import software.amazon.cloudwatchlogs.emf.Constants;
2122
import software.amazon.cloudwatchlogs.emf.config.Configuration;
@@ -37,11 +38,14 @@ public boolean probe() {
3738

3839
@Override
3940
public String getType() {
40-
if (config.getServiceType().isEmpty()) {
41-
log.info("Unknown ServiceType");
42-
return Constants.UNKNOWN;
41+
Optional<String> serviceType = config.getServiceType();
42+
43+
if (serviceType.isPresent()) {
44+
return serviceType.get();
4345
}
44-
return config.getServiceType().get();
46+
47+
log.info("Unknown ServiceType");
48+
return Constants.UNKNOWN;
4549
}
4650

4751
@Override

‎src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ MetricDirective copyWithoutMetrics(boolean preserveDimensions) {
168168
metricDirective.shouldUseDefaultDimension = this.shouldUseDefaultDimension;
169169

170170
if (preserveDimensions) {
171-
metricDirective.setDimensions(this.dimensions);
171+
this.dimensions.forEach(metricDirective::putDimensionSet);
172172
}
173173

174174
return metricDirective;

‎src/test/java/software/amazon/cloudwatchlogs/emf/logger/MetricsLoggerTest.java

+34-8
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.mockito.Mockito.*;
2121

2222
import java.time.Instant;
23+
import java.util.Arrays;
2324
import java.util.List;
2425
import java.util.Set;
2526
import java.util.concurrent.CompletableFuture;
@@ -93,8 +94,8 @@ void whenDefaultDimension_DimensionValue_Empty() throws DimensionSetExceededExce
9394
assertEquals(1, sink.getContext().getDimensions().size());
9495
Set<String> dimensionKeys = sink.getContext().getDimensions().get(0).getDimensionKeys();
9596
assertEquals(2, dimensionKeys.size());
96-
dimensionKeys.contains("ServiceName");
97-
dimensionKeys.contains("ServiceType");
97+
assertTrue(dimensionKeys.contains("ServiceName"));
98+
assertTrue(dimensionKeys.contains("ServiceType"));
9899
}
99100

100101
@Test
@@ -105,8 +106,8 @@ void whenDefaultDimension_DimensionValue_Blank() throws DimensionSetExceededExce
105106
assertEquals(1, sink.getContext().getDimensions().size());
106107
Set<String> dimensionKeys = sink.getContext().getDimensions().get(0).getDimensionKeys();
107108
assertEquals(2, dimensionKeys.size());
108-
dimensionKeys.contains("ServiceName");
109-
dimensionKeys.contains("ServiceType");
109+
assertTrue(dimensionKeys.contains("ServiceName"));
110+
assertTrue(dimensionKeys.contains("ServiceType"));
110111
}
111112

112113
@ParameterizedTest
@@ -126,8 +127,11 @@ void whenSetDimension_withInvalidValue_thenThrowInvalidDimensionException(
126127

127128
@Test
128129
void whenSetDimension_withNameTooLong_thenThrowDimensionException() {
129-
String dimensionName = "a".repeat(Constants.MAX_DIMENSION_NAME_LENGTH + 1);
130+
char[] longName = new char[Constants.MAX_DIMENSION_NAME_LENGTH + 1];
131+
Arrays.fill(longName, 'a');
132+
String dimensionName = String.valueOf(longName);
130133
String dimensionValue = "dimValue";
134+
131135
assertThrows(
132136
InvalidDimensionException.class,
133137
() -> DimensionSet.of(dimensionName, dimensionValue));
@@ -136,7 +140,10 @@ void whenSetDimension_withNameTooLong_thenThrowDimensionException() {
136140
@Test
137141
void whenSetDimension_withValueTooLong_thenThrowDimensionException() {
138142
String dimensionName = "dim";
139-
String dimensionValue = "a".repeat(Constants.MAX_DIMENSION_VALUE_LENGTH + 1);
143+
char[] longName = new char[Constants.MAX_DIMENSION_VALUE_LENGTH + 1];
144+
Arrays.fill(longName, 'a');
145+
String dimensionValue = String.valueOf(longName);
146+
140147
assertThrows(
141148
InvalidDimensionException.class,
142149
() -> DimensionSet.of(dimensionName, dimensionValue));
@@ -275,6 +282,19 @@ void flush_doesNotPreserveDimensions()
275282
expectDimension("Name", null);
276283
}
277284

285+
@Test
286+
void whenMultipleFlush_withNoDimensionsChanges_keepDefaultDimensions()
287+
throws InvalidMetricException, DimensionSetExceededException {
288+
logger.putMetric("Count", 1);
289+
290+
logger.flush();
291+
logger.flush();
292+
293+
expectDimension("LogGroup", "test-log-group");
294+
expectDimension("ServiceName", "test-env-name");
295+
expectDimension("ServiceType", "test-env-type");
296+
}
297+
278298
@Test
279299
void setDimensions_clearsAllDimensions() throws DimensionSetExceededException {
280300
MetricsLogger logger = new MetricsLogger(envProvider);
@@ -302,7 +322,10 @@ void whenSetDimensions_withMultipleFlush_thenClearsDimensions()
302322

303323
@Test
304324
void whenPutMetric_withTooLongName_thenThrowInvalidMetricException() {
305-
String name1 = "a".repeat(Constants.MAX_METRIC_NAME_LENGTH + 1);
325+
char[] longName = new char[Constants.MAX_METRIC_NAME_LENGTH + 1];
326+
Arrays.fill(longName, 'a');
327+
String name1 = new String(longName);
328+
306329
assertThrows(InvalidMetricException.class, () -> logger.putMetric(name1, 1));
307330
}
308331

@@ -349,7 +372,10 @@ void whenSetNamespace_withInvalidValue_thenThrowInvalidNamespaceException(String
349372

350373
@Test
351374
void whenSetNamespace_withNameTooLong_thenThrowInvalidNamespaceException() {
352-
String namespace = "a".repeat(Constants.MAX_NAMESPACE_LENGTH + 1);
375+
char[] longName = new char[Constants.MAX_NAMESPACE_LENGTH + 1];
376+
Arrays.fill(longName, 'a');
377+
String namespace = new String(longName);
378+
353379
assertThrows(InvalidNamespaceException.class, () -> logger.setNamespace(namespace));
354380
}
355381

‎src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionTest.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020

2121
import com.fasterxml.jackson.core.JsonProcessingException;
2222
import com.fasterxml.jackson.databind.ObjectMapper;
23-
import java.util.List;
23+
import java.util.Arrays;
24+
import java.util.Collections;
2425
import org.junit.Test;
2526

2627
public class MetricDefinitionTest {
@@ -51,9 +52,9 @@ public void testSerializeMetricDefinition() throws JsonProcessingException {
5152
@Test
5253
public void testAddValue() {
5354
MetricDefinition md = new MetricDefinition("Time", Unit.MICROSECONDS, 10);
54-
assertEquals(List.of(10d), md.getValues());
55+
assertEquals(Collections.singletonList(10d), md.getValues());
5556

5657
md.addValue(20);
57-
assertEquals(List.of(10d, 20d), md.getValues());
58+
assertEquals(Arrays.asList(10d, 20d), md.getValues());
5859
}
5960
}

‎src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricsContextTest.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.fasterxml.jackson.databind.json.JsonMapper;
2222
import java.time.Instant;
2323
import java.util.ArrayList;
24+
import java.util.Collections;
2425
import java.util.List;
2526
import java.util.Map;
2627
import org.junit.jupiter.api.Assertions;
@@ -101,7 +102,7 @@ void testSerializeAMetricWith101DataPoints()
101102
expectedValues.add((double) i);
102103
}
103104
Assertions.assertEquals(expectedValues, allMetrics.get(0).getValues());
104-
Assertions.assertEquals(List.of(100.0), allMetrics.get(1).getValues());
105+
Assertions.assertEquals(Collections.singletonList(100.0), allMetrics.get(1).getValues());
105106
}
106107

107108
@Test
@@ -128,10 +129,12 @@ void testSerializeMetricsWith101DataPoints()
128129
expectedValues.add((double) i);
129130
}
130131
Assertions.assertEquals(expectedValues, metricsFromEvent1.get(0).getValues());
131-
Assertions.assertEquals(List.of(2.0), metricsFromEvent1.get(1).getValues());
132+
Assertions.assertEquals(
133+
Collections.singletonList(2.0), metricsFromEvent1.get(1).getValues());
132134

133135
Assertions.assertEquals(1, metricsFromEvent2.size());
134-
Assertions.assertEquals(List.of(100.0), metricsFromEvent2.get(0).getValues());
136+
Assertions.assertEquals(
137+
Collections.singletonList(100.0), metricsFromEvent2.get(0).getValues());
135138
}
136139

137140
@Test

‎src/test/java/software/amazon/cloudwatchlogs/emf/model/RootNodeTest.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.fasterxml.jackson.core.JsonProcessingException;
2020
import com.fasterxml.jackson.core.type.TypeReference;
2121
import com.fasterxml.jackson.databind.ObjectMapper;
22+
import java.util.Arrays;
2223
import java.util.List;
2324
import java.util.Map;
2425
import org.junit.jupiter.api.Assertions;
@@ -72,7 +73,8 @@ void testGetTargetMembers()
7273

7374
mc.putProperty("Prop1", "PropValue1");
7475

75-
Assertions.assertEquals(List.of(10.0, 20.0), rootNode.getTargetMembers().get("Count"));
76+
Assertions.assertEquals(
77+
Arrays.asList(10.0, 20.0), rootNode.getTargetMembers().get("Count"));
7678
Assertions.assertEquals(100.0, rootNode.getTargetMembers().get("Latency"));
7779
Assertions.assertEquals("DimVal1", rootNode.getTargetMembers().get("Dim1"));
7880
Assertions.assertEquals("PropValue1", rootNode.getTargetMembers().get("Prop1"));

‎src/test/java/software/amazon/cloudwatchlogs/emf/sinks/AgentSinkTest.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ public void testEmptyLogGroupName() throws JsonProcessingException, InvalidMetri
103103
ObjectMapper objectMapper = new ObjectMapper();
104104
Map<String, Object> emf_map =
105105
objectMapper.readValue(
106-
fixture.client.getMessages().get(0), new TypeReference<>() {});
106+
fixture.client.getMessages().get(0),
107+
new TypeReference<Map<String, Object>>() {});
107108
Map<String, Object> metadata = (Map<String, Object>) emf_map.get("_aws");
108109

109110
assertFalse(metadata.containsKey("LogGroupName"));

‎src/test/java/software/amazon/cloudwatchlogs/emf/sinks/MultiSinkTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ public void shutdownClosesAllComponentSinks() {
2828
@Test
2929
public void shutdownCompletesExceptionallyIfComponentSinkCompletesExceptionally() {
3030
// arrange
31-
CompletableFuture<Void> failedResult =
32-
CompletableFuture.failedFuture(new RuntimeException());
31+
CompletableFuture<Void> failedResult = new CompletableFuture<>();
32+
failedResult.completeExceptionally(new RuntimeException());
3333
TestSink sink1 = new TestSink();
3434
TestSink sink2 = new TestSink(failedResult);
3535
MultiSink multiSink = MultiSink.builder().sink(sink1).sink(sink2).build();

0 commit comments

Comments
 (0)