Skip to content

Commit 1f831de

Browse files
Brianwithay21Brianwithay21
authored
Remove hooks platform metrics (#394)
* Removed publishing platform metrics for HOOK types * Remove unnecessary model validation in Hook Wrapper * Fix bug with loading a hook's target schema
1 parent ed4377f commit 1f831de

File tree

12 files changed

+46
-269
lines changed

12 files changed

+46
-269
lines changed

‎python/rpdk/java/templates/generate/hook/BaseHookConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public JSONObject targetSchemaJSONObject(final String targetName) {
4444
new JSONTokener(
4545
this.targetSchemas.computeIfAbsent(
4646
targetName,
47-
tn -> this.getClass().getClassLoader().getResourceAsStream(tn)
47+
tn -> this.getClass().getClassLoader().getResourceAsStream(this.targetSchemaPaths.get(tn))
4848
)
4949
)
5050
);

‎src/main/java/software/amazon/cloudformation/HookAbstractWrapper.java

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import software.amazon.awssdk.http.HttpStatusFamily;
3636
import software.amazon.awssdk.http.SdkHttpClient;
3737
import software.amazon.awssdk.http.apache.ApacheHttpClient;
38-
import software.amazon.awssdk.utils.StringUtils;
3938
import software.amazon.cloudformation.encryption.Cipher;
4039
import software.amazon.cloudformation.encryption.KMSCipher;
4140
import software.amazon.cloudformation.exceptions.BaseHandlerException;
@@ -67,7 +66,6 @@
6766
import software.amazon.cloudformation.resource.SchemaValidator;
6867
import software.amazon.cloudformation.resource.Serializer;
6968
import software.amazon.cloudformation.resource.Validator;
70-
import software.amazon.cloudformation.resource.exceptions.ValidationException;
7169

7270
public abstract class HookAbstractWrapper<TargetT, CallbackT, ConfigurationT> {
7371

@@ -90,7 +88,6 @@ public abstract class HookAbstractWrapper<TargetT, CallbackT, ConfigurationT> {
9088
final SchemaValidator validator;
9189
final TypeReference<HookInvocationRequest<ConfigurationT, CallbackT>> typeReference;
9290

93-
private MetricsPublisher platformMetricsPublisher;
9491
private MetricsPublisher providerMetricsPublisher;
9592

9693
private CloudWatchLogHelper cloudWatchLogHelper;
@@ -112,7 +109,6 @@ protected HookAbstractWrapper() {
112109
public HookAbstractWrapper(final CredentialsProvider providerCredentialsProvider,
113110
final CloudWatchLogPublisher providerEventsLogger,
114111
final LogPublisher platformEventsLogger,
115-
final MetricsPublisher platformMetricsPublisher,
116112
final MetricsPublisher providerMetricsPublisher,
117113
final SchemaValidator validator,
118114
final Serializer serializer,
@@ -123,7 +119,6 @@ public HookAbstractWrapper(final CredentialsProvider providerCredentialsProvider
123119
this.cloudWatchLogsProvider = new CloudWatchLogsProvider(this.providerCredentialsProvider, httpClient);
124120
this.providerEventsLogger = providerEventsLogger;
125121
this.platformLogPublisher = platformEventsLogger;
126-
this.platformMetricsPublisher = platformMetricsPublisher;
127122
this.providerMetricsPublisher = providerMetricsPublisher;
128123
this.serializer = serializer;
129124
this.validator = validator;
@@ -147,22 +142,14 @@ private void initialiseRuntime(final String hookTypeName,
147142
this.loggerProxy = new LoggerProxy();
148143
this.loggerProxy.addLogPublisher(this.platformLogPublisher);
149144

145+
// Initialisation skipped if dependencies were set during injection (in unit
146+
// tests).
147+
150148
// Initialize a KMS cipher to decrypt customer credentials in HookRequestData
151149
if (this.cipher == null && hookEncryptionKeyArn != null && hookEncryptionKeyRole != null) {
152150
this.cipher = new KMSCipher(hookEncryptionKeyArn, hookEncryptionKeyRole);
153151
}
154152

155-
// Initialisation skipped if dependencies were set during injection (in unit
156-
// tests).
157-
// e.g. "if (this.platformMetricsPublisher == null)"
158-
if (this.platformMetricsPublisher == null) {
159-
// platformMetricsPublisher needs aws account id to differentiate metrics
160-
// namespace
161-
this.platformMetricsPublisher = new HookMetricsPublisherImpl(this.platformLoggerProxy, awsAccountId, hookTypeName);
162-
}
163-
this.metricsPublisherProxy.addMetricsPublisher(this.platformMetricsPublisher);
164-
this.platformMetricsPublisher.refreshClient();
165-
166153
// NOTE: providerCredentials and providerLogGroupName are null/not null in
167154
// sync.
168155
// Both are required parameters when LoggingConfig (optional) is provided when
@@ -210,19 +197,6 @@ public void processRequest(final InputStream inputStream, final OutputStream out
210197
// deserialize incoming payload to modeled request
211198
request = this.serializer.deserialize(input, typeReference);
212199
handlerResponse = processInvocation(rawInput, request);
213-
} catch (final ValidationException e) {
214-
String message;
215-
String fullExceptionMessage = ValidationException.buildFullExceptionMessage(e);
216-
if (!StringUtils.isEmpty(fullExceptionMessage)) {
217-
message = String.format("Model validation failed (%s)", fullExceptionMessage);
218-
} else {
219-
message = "Model validation failed with unknown cause.";
220-
}
221-
222-
handlerResponse = ProgressEvent.defaultFailureHandler(new TerminalException(message, e),
223-
HandlerErrorCode.InvalidRequest);
224-
publishExceptionMetric(request != null ? request.getActionInvocationPoint() : null, e,
225-
HandlerErrorCode.InvalidRequest);
226200
} catch (final Throwable e) {
227201
// Exceptions are wrapped as a consistent error response to the caller (i.e;
228202
// CloudFormation)

‎src/main/java/software/amazon/cloudformation/HookExecutableWrapper.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,13 @@ public HookExecutableWrapper() {
4444
public HookExecutableWrapper(final CredentialsProvider providerCredentialsProvider,
4545
final CloudWatchLogPublisher providerEventsLogger,
4646
final LogPublisher platformEventsLogger,
47-
final MetricsPublisher platformMetricsPublisher,
4847
final MetricsPublisher providerMetricsPublisher,
4948
final SchemaValidator validator,
5049
final Serializer serializer,
5150
final SdkHttpClient httpClient,
5251
final Cipher cipher) {
53-
super(providerCredentialsProvider, providerEventsLogger, platformEventsLogger, platformMetricsPublisher,
54-
providerMetricsPublisher, validator, serializer, httpClient, cipher);
52+
super(providerCredentialsProvider, providerEventsLogger, platformEventsLogger, providerMetricsPublisher, validator,
53+
serializer, httpClient, cipher);
5554
}
5655

5756
public void handleRequest(final InputStream inputStream, final OutputStream outputStream) throws IOException,

‎src/main/java/software/amazon/cloudformation/HookLambdaWrapper.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,13 @@ public HookLambdaWrapper() {
4343
public HookLambdaWrapper(final CredentialsProvider providerCredentialsProvider,
4444
final CloudWatchLogPublisher providerEventsLogger,
4545
final LogPublisher platformEventsLogger,
46-
final MetricsPublisher platformMetricsPublisher,
4746
final MetricsPublisher providerMetricsPublisher,
4847
final SchemaValidator validator,
4948
final Serializer serializer,
5049
final SdkHttpClient httpClient,
5150
final Cipher cipher) {
52-
super(providerCredentialsProvider, providerEventsLogger, platformEventsLogger, platformMetricsPublisher,
53-
providerMetricsPublisher, validator, serializer, httpClient, cipher);
51+
super(providerCredentialsProvider, providerEventsLogger, platformEventsLogger, providerMetricsPublisher, validator,
52+
serializer, httpClient, cipher);
5453
}
5554

5655
@Override

‎src/main/java/software/amazon/cloudformation/metrics/HookMetricsPublisherImpl.java

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import java.util.Collection;
2020
import java.util.EnumSet;
2121
import java.util.HashSet;
22-
import software.amazon.awssdk.core.SdkSystemSetting;
23-
import software.amazon.awssdk.regions.Region;
2422
import software.amazon.awssdk.services.cloudwatch.CloudWatchClient;
2523
import software.amazon.awssdk.services.cloudwatch.model.Dimension;
2624
import software.amazon.awssdk.services.cloudwatch.model.MetricDatum;
@@ -32,9 +30,7 @@
3230
import software.amazon.cloudformation.proxy.Logger;
3331

3432
public class HookMetricsPublisherImpl extends MetricsPublisher {
35-
private static final String DEFAULT_REGION = "us-east-1";
36-
37-
private CloudWatchProvider cloudWatchProvider;
33+
private final CloudWatchProvider cloudWatchProvider;
3834
private Logger loggerProxy;
3935
private String awsAccountId;
4036
private CloudWatchClient cloudWatchClient;
@@ -49,20 +45,9 @@ public HookMetricsPublisherImpl(final CloudWatchProvider cloudWatchProvider,
4945
this.awsAccountId = awsAccountId;
5046
}
5147

52-
public HookMetricsPublisherImpl(final Logger loggerProxy,
53-
final String awsAccountId,
54-
final String hookTypeName) {
55-
super(hookTypeName);
56-
this.loggerProxy = loggerProxy;
57-
this.awsAccountId = awsAccountId;
58-
this.cloudWatchClient = createClient();
59-
}
60-
6148
@Override
6249
public void refreshClient() {
63-
if (cloudWatchProvider != null) {
64-
this.cloudWatchClient = cloudWatchProvider.get();
65-
}
50+
this.cloudWatchClient = cloudWatchProvider.get();
6651
}
6752

6853
private String getHookTypeName() {
@@ -166,9 +151,4 @@ private void log(final String message) {
166151
}
167152
}
168153

169-
private CloudWatchClient createClient() {
170-
final String region = SdkSystemSetting.AWS_REGION.getStringValue().map(Object::toString).orElse(DEFAULT_REGION);
171-
return CloudWatchClient.builder().region(Region.of(region)).build();
172-
}
173-
174154
}

‎src/test/java/software/amazon/cloudformation/HookExecutableWrapperOverride.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,12 @@ public class HookExecutableWrapperOverride extends HookExecutableWrapper<TestMod
5050
public HookExecutableWrapperOverride(final CredentialsProvider providerLoggingCredentialsProvider,
5151
final LogPublisher platformEventsLogger,
5252
final CloudWatchLogPublisher providerEventsLogger,
53-
final MetricsPublisher platformMetricsPublisher,
5453
final MetricsPublisher providerMetricsPublisher,
5554
final SchemaValidator validator,
5655
final SdkHttpClient httpClient,
5756
final Cipher cipher) {
58-
super(providerLoggingCredentialsProvider, providerEventsLogger, platformEventsLogger, platformMetricsPublisher,
59-
providerMetricsPublisher, validator, new Serializer(), httpClient, cipher);
57+
super(providerLoggingCredentialsProvider, providerEventsLogger, platformEventsLogger, providerMetricsPublisher, validator,
58+
new Serializer(), httpClient, cipher);
6059
}
6160

6261
@Override

‎src/test/java/software/amazon/cloudformation/HookExecutableWrapperTest.java

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,8 @@
1616

1717
import static org.assertj.core.api.Assertions.assertThat;
1818
import static org.mockito.ArgumentMatchers.any;
19-
import static org.mockito.ArgumentMatchers.anyLong;
20-
import static org.mockito.ArgumentMatchers.eq;
21-
import static org.mockito.ArgumentMatchers.isNull;
2219
import static org.mockito.Mockito.lenient;
23-
import static org.mockito.Mockito.times;
2420
import static org.mockito.Mockito.verify;
25-
import static org.mockito.Mockito.verifyNoMoreInteractions;
2621
import com.fasterxml.jackson.core.type.TypeReference;
2722
import java.io.ByteArrayOutputStream;
2823
import java.io.File;
@@ -31,7 +26,6 @@
3126
import java.io.IOException;
3227
import java.io.InputStream;
3328
import java.io.OutputStream;
34-
import java.time.Instant;
3529
import org.junit.jupiter.api.BeforeEach;
3630
import org.junit.jupiter.api.extension.ExtendWith;
3731
import org.junit.jupiter.params.ParameterizedTest;
@@ -61,9 +55,6 @@ public class HookExecutableWrapperTest {
6155
@Mock
6256
private CredentialsProvider providerLoggingCredentialsProvider;
6357

64-
@Mock
65-
private MetricsPublisher platformMetricsPublisher;
66-
6758
@Mock
6859
private MetricsPublisher providerMetricsPublisher;
6960

@@ -90,8 +81,8 @@ public class HookExecutableWrapperTest {
9081
@BeforeEach
9182
public void initWrapper() {
9283
wrapper = new HookExecutableWrapperOverride(providerLoggingCredentialsProvider, platformEventsLogger,
93-
providerEventsLogger, platformMetricsPublisher, providerMetricsPublisher,
94-
validator, httpClient, cipher);
84+
providerEventsLogger, providerMetricsPublisher, validator, httpClient,
85+
cipher);
9586
}
9687

9788
private static InputStream loadRequestStream(final String fileName) {
@@ -107,7 +98,6 @@ private static InputStream loadRequestStream(final String fileName) {
10798

10899
private void verifyInitialiseRuntime() {
109100
verify(providerLoggingCredentialsProvider).setCredentials(any(Credentials.class));
110-
verify(platformMetricsPublisher).refreshClient();
111101
verify(providerMetricsPublisher).refreshClient();
112102
}
113103

@@ -151,15 +141,6 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
151141
// verify initialiseRuntime was called and initialised dependencies
152142
verifyInitialiseRuntime();
153143

154-
// all metrics should be published, once for a single invocation
155-
verify(platformMetricsPublisher, times(1)).publishInvocationMetric(any(Instant.class), eq(invocationPoint));
156-
verify(platformMetricsPublisher, times(1)).publishDurationMetric(any(Instant.class), eq(invocationPoint), anyLong());
157-
verify(platformMetricsPublisher, times(1)).publishExceptionByErrorCodeAndCountBulkMetrics(any(Instant.class),
158-
any(HookInvocationPoint.class), isNull());
159-
160-
// validation failure metric should not be published
161-
verifyNoMoreInteractions(platformMetricsPublisher);
162-
163144
// verify output response
164145
verifyHandlerResponse(out,
165146
HookProgressEvent.<TestContext>builder().clientRequestToken("123456").hookStatus(HookStatus.SUCCESS).build());

‎src/test/java/software/amazon/cloudformation/HookLambdaWrapperOverride.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,12 @@ public class HookLambdaWrapperOverride extends HookLambdaWrapper<TestModel, Test
5050
public HookLambdaWrapperOverride(final CredentialsProvider providerLoggingCredentialsProvider,
5151
final LogPublisher platformEventsLogger,
5252
final CloudWatchLogPublisher providerEventsLogger,
53-
final MetricsPublisher platformMetricsPublisher,
5453
final MetricsPublisher providerMetricsPublisher,
5554
final SchemaValidator validator,
5655
final SdkHttpClient httpClient,
5756
final Cipher cipher) {
58-
super(providerLoggingCredentialsProvider, providerEventsLogger, platformEventsLogger, platformMetricsPublisher,
59-
providerMetricsPublisher, validator, new Serializer(), httpClient, cipher);
57+
super(providerLoggingCredentialsProvider, providerEventsLogger, platformEventsLogger, providerMetricsPublisher, validator,
58+
new Serializer(), httpClient, cipher);
6059
}
6160

6261
@Override

‎src/test/java/software/amazon/cloudformation/HookLambdaWrapperTest.java

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,9 @@
1616

1717
import static org.assertj.core.api.Assertions.assertThat;
1818
import static org.mockito.ArgumentMatchers.any;
19-
import static org.mockito.ArgumentMatchers.anyLong;
20-
import static org.mockito.ArgumentMatchers.eq;
21-
import static org.mockito.ArgumentMatchers.isNull;
2219
import static org.mockito.Mockito.lenient;
2320
import static org.mockito.Mockito.mock;
24-
import static org.mockito.Mockito.times;
2521
import static org.mockito.Mockito.verify;
26-
import static org.mockito.Mockito.verifyNoMoreInteractions;
2722
import com.amazonaws.services.lambda.runtime.Context;
2823
import com.amazonaws.services.lambda.runtime.LambdaLogger;
2924
import com.fasterxml.jackson.core.type.TypeReference;
@@ -34,7 +29,6 @@
3429
import java.io.IOException;
3530
import java.io.InputStream;
3631
import java.io.OutputStream;
37-
import java.time.Instant;
3832
import org.junit.jupiter.api.BeforeEach;
3933
import org.junit.jupiter.api.extension.ExtendWith;
4034
import org.junit.jupiter.params.ParameterizedTest;
@@ -64,9 +58,6 @@ public class HookLambdaWrapperTest {
6458
@Mock
6559
private CredentialsProvider providerLoggingCredentialsProvider;
6660

67-
@Mock
68-
private MetricsPublisher platformMetricsPublisher;
69-
7061
@Mock
7162
private MetricsPublisher providerMetricsPublisher;
7263

@@ -96,8 +87,7 @@ public class HookLambdaWrapperTest {
9687
@BeforeEach
9788
public void initWrapper() {
9889
wrapper = new HookLambdaWrapperOverride(providerLoggingCredentialsProvider, platformEventsLogger, providerEventsLogger,
99-
platformMetricsPublisher, providerMetricsPublisher, validator, httpClient,
100-
cipher);
90+
providerMetricsPublisher, validator, httpClient, cipher);
10191
}
10292

10393
private static InputStream loadRequestStream(final String fileName) {
@@ -121,7 +111,6 @@ private Context getLambdaContext() {
121111

122112
private void verifyInitialiseRuntime() {
123113
verify(providerLoggingCredentialsProvider).setCredentials(any(Credentials.class));
124-
verify(platformMetricsPublisher).refreshClient();
125114
verify(providerMetricsPublisher).refreshClient();
126115
}
127116

@@ -166,15 +155,6 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
166155
// verify initialiseRuntime was called and initialised dependencies
167156
verifyInitialiseRuntime();
168157

169-
// all metrics should be published, once for a single invocation
170-
verify(platformMetricsPublisher, times(1)).publishInvocationMetric(any(Instant.class), eq(invocationPoint));
171-
verify(platformMetricsPublisher, times(1)).publishDurationMetric(any(Instant.class), eq(invocationPoint), anyLong());
172-
verify(platformMetricsPublisher, times(1)).publishExceptionByErrorCodeAndCountBulkMetrics(any(Instant.class),
173-
any(HookInvocationPoint.class), isNull());
174-
175-
// validation failure metric should not be published
176-
verifyNoMoreInteractions(platformMetricsPublisher);
177-
178158
// verify output response
179159
verifyHandlerResponse(out,
180160
HookProgressEvent.<TestContext>builder().clientRequestToken("123456").hookStatus(HookStatus.SUCCESS).build());

‎src/test/java/software/amazon/cloudformation/HookWrapperOverride.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,12 @@ public HookWrapperOverride(final LogPublisher platformEventsLogger,
6666
public HookWrapperOverride(final CredentialsProvider providerLoggingCredentialsProvider,
6767
final LogPublisher platformEventsLogger,
6868
final CloudWatchLogPublisher providerEventsLogger,
69-
final MetricsPublisher platformMetricsPublisher,
7069
final MetricsPublisher providerMetricsPublisher,
7170
final SchemaValidator validator,
7271
final SdkHttpClient httpClient,
7372
final Cipher cipher) {
74-
super(providerLoggingCredentialsProvider, providerEventsLogger, platformEventsLogger, platformMetricsPublisher,
75-
providerMetricsPublisher, validator, new Serializer(), httpClient, cipher);
73+
super(providerLoggingCredentialsProvider, providerEventsLogger, platformEventsLogger, providerMetricsPublisher, validator,
74+
new Serializer(), httpClient, cipher);
7675
}
7776

7877
@Override

0 commit comments

Comments
 (0)