Skip to content

Commit e13f80a

Browse files
committed
Address code review feedback:
1. Added a null check at the awsResp level. 2. Removed the call to getARN() from the request. In Secrets Manager, the request uses secretId, which is not necessarily an ARN. This change aligns with the V2 implementation. The ARN will now be retrieved from the response. A new test has been added using DescribeSecretRequest.
1 parent 87aae70 commit e13f80a

File tree

2 files changed

+41
-5
lines changed

2 files changed

+41
-5
lines changed

instrumentation/aws-sdk/aws-sdk-1.11/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkAttributesExtractor.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ private static boolean canGetResponseMetadata() {
4545
@Override
4646
public void onStart(AttributesBuilder attributes, Context parentContext, Request<?> request) {
4747
Object originalRequest = request.getOriginalRequest();
48-
setAttribute(
49-
attributes, AWS_SECRETSMANAGER_SECRET_ARN, originalRequest, RequestAccess::getSecretArn);
5048
setAttribute(
5149
attributes,
5250
AWS_STEP_FUNCTIONS_STATE_MACHINE_ARN,
@@ -66,8 +64,8 @@ public void onEnd(
6664
Request<?> request,
6765
@Nullable Response<?> response,
6866
@Nullable Throwable error) {
69-
if (response != null) {
70-
Object awsResp = response.getAwsResponse();
67+
Object awsResp = getAwsResponse(response);
68+
if (awsResp != null) {
7169
setAttribute(attributes, AWS_SECRETSMANAGER_SECRET_ARN, awsResp, RequestAccess::getSecretArn);
7270
setAttribute(
7371
attributes,
@@ -111,4 +109,11 @@ public static void setAttribute(
111109
attributes.put(key, value);
112110
}
113111
}
112+
113+
private static Object getAwsResponse(Response<?> response) {
114+
if (response == null) {
115+
return null;
116+
}
117+
return response.getAwsResponse();
118+
}
114119
}

instrumentation/aws-sdk/aws-sdk-1.11/testing/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSecretsManagerClientTest.java

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.amazonaws.services.secretsmanager.AWSSecretsManager;
1313
import com.amazonaws.services.secretsmanager.AWSSecretsManagerClientBuilder;
1414
import com.amazonaws.services.secretsmanager.model.CreateSecretRequest;
15+
import com.amazonaws.services.secretsmanager.model.DescribeSecretRequest;
1516
import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
1617
import io.opentelemetry.testing.internal.armeria.common.HttpResponse;
1718
import io.opentelemetry.testing.internal.armeria.common.HttpStatus;
@@ -30,7 +31,7 @@ protected boolean hasRequestId() {
3031
}
3132

3233
@Test
33-
public void sendRequestWithMockedResponse() throws Exception {
34+
public void sendCreateSecretRequestWithMockedResponse() throws Exception {
3435
AWSSecretsManagerClientBuilder clientBuilder = AWSSecretsManagerClientBuilder.standard();
3536
AWSSecretsManager client =
3637
configureClient(clientBuilder)
@@ -59,4 +60,34 @@ public void sendRequestWithMockedResponse() throws Exception {
5960
assertRequestWithMockedResponse(
6061
response, client, "AWSSecretsManager", "CreateSecret", "POST", additionalAttributes);
6162
}
63+
64+
@Test
65+
public void sendDescribeSecretRequestWithMockedResponse() throws Exception {
66+
AWSSecretsManagerClientBuilder clientBuilder = AWSSecretsManagerClientBuilder.standard();
67+
AWSSecretsManager client =
68+
configureClient(clientBuilder)
69+
.withEndpointConfiguration(endpoint)
70+
.withCredentials(credentialsProvider)
71+
.build();
72+
73+
String body =
74+
"{"
75+
+ "\"ARN\": \"arn:aws:secretsmanager:us-east-1:123456789012:secret:My-Secret-Id-WzAXar\","
76+
+ "\"Name\": \"My-Secret-Id\","
77+
+ "\"VersionId\": \"EXAMPLE1-90ab-cdef-fedc-ba987SECRET1\""
78+
+ "}";
79+
80+
server.enqueue(HttpResponse.of(HttpStatus.OK, MediaType.PLAIN_TEXT_UTF_8, body));
81+
Object response = client.describeSecret(
82+
new DescribeSecretRequest().withSecretId("My-Secret-Id"));
83+
84+
List<AttributeAssertion> additionalAttributes =
85+
singletonList(
86+
equalTo(
87+
AWS_SECRETSMANAGER_SECRET_ARN,
88+
"arn:aws:secretsmanager:us-east-1:123456789012:secret:My-Secret-Id-WzAXar"));
89+
90+
assertRequestWithMockedResponse(
91+
response, client, "AWSSecretsManager", "DescribeSecret", "POST", additionalAttributes);
92+
}
6293
}

0 commit comments

Comments
 (0)