-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: Add auto-instrumentation support for AWS Secrets Manager SDK v1 #14027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
87aae70
e13f80a
7d9d617
5ab1706
6fd5587
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,8 @@ class AwsSdkAttributesExtractor implements AttributesExtractor<Request<?>, Respo | |
| private static final AttributeKey<String> AWS_REQUEST_ID = stringKey("aws.request_id"); | ||
|
|
||
| // Copied from AwsIncubatingAttributes | ||
| private static final AttributeKey<String> AWS_SECRETSMANAGER_SECRET_ARN = | ||
| stringKey("aws.secretsmanager.secret.arn"); | ||
| private static final AttributeKey<String> AWS_STEP_FUNCTIONS_ACTIVITY_ARN = | ||
| stringKey("aws.step_functions.activity.arn"); | ||
| private static final AttributeKey<String> AWS_STEP_FUNCTIONS_STATE_MACHINE_ARN = | ||
|
|
@@ -62,8 +64,9 @@ public void onEnd( | |
| Request<?> request, | ||
| @Nullable Response<?> response, | ||
| @Nullable Throwable error) { | ||
| if (response != null) { | ||
| Object awsResp = response.getAwsResponse(); | ||
| Object awsResp = getAwsResponse(response); | ||
| if (awsResp != null) { | ||
| setAttribute(attributes, AWS_SECRETSMANAGER_SECRET_ARN, awsResp, RequestAccess::getSecretArn); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps there should be a null check for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are great feedback. Thank you @laurit.
|
||
| setAttribute( | ||
| attributes, | ||
| AWS_STEP_FUNCTIONS_STATE_MACHINE_ARN, | ||
|
|
@@ -106,4 +109,11 @@ public static void setAttribute( | |
| attributes.put(key, value); | ||
| } | ||
| } | ||
|
|
||
| private static Object getAwsResponse(Response<?> response) { | ||
| if (response == null) { | ||
| return null; | ||
| } | ||
| return response.getAwsResponse(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.instrumentation.awssdk.v1_11; | ||
|
|
||
| import com.amazonaws.services.secretsmanager.AWSSecretsManagerClientBuilder; | ||
| import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; | ||
| import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; | ||
| import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
|
||
| class SecretsManagerClientTest extends AbstractSecretsManagerClientTest { | ||
| @RegisterExtension | ||
| private static final InstrumentationExtension testing = LibraryInstrumentationExtension.create(); | ||
|
|
||
| @Override | ||
| protected InstrumentationExtension testing() { | ||
| return testing; | ||
| } | ||
|
|
||
| @Override | ||
| public AWSSecretsManagerClientBuilder configureClient( | ||
| AWSSecretsManagerClientBuilder clientBuilder) { | ||
|
|
||
| return clientBuilder.withRequestHandlers( | ||
| AwsSdkTelemetry.builder(testing().getOpenTelemetry()) | ||
| .setCaptureExperimentalSpanAttributes(true) | ||
| .build() | ||
| .newRequestHandler()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.instrumentation.awssdk.v1_11; | ||
|
|
||
| import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; | ||
| import static io.opentelemetry.semconv.incubating.AwsIncubatingAttributes.AWS_SECRETSMANAGER_SECRET_ARN; | ||
| import static java.util.Collections.singletonList; | ||
|
|
||
| import com.amazonaws.services.secretsmanager.AWSSecretsManager; | ||
| import com.amazonaws.services.secretsmanager.AWSSecretsManagerClientBuilder; | ||
| import com.amazonaws.services.secretsmanager.model.CreateSecretRequest; | ||
| import com.amazonaws.services.secretsmanager.model.DescribeSecretRequest; | ||
| import io.opentelemetry.sdk.testing.assertj.AttributeAssertion; | ||
| import io.opentelemetry.testing.internal.armeria.common.HttpResponse; | ||
| import io.opentelemetry.testing.internal.armeria.common.HttpStatus; | ||
| import io.opentelemetry.testing.internal.armeria.common.MediaType; | ||
| import java.util.List; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public abstract class AbstractSecretsManagerClientTest extends AbstractBaseAwsClientTest { | ||
|
|
||
| public abstract AWSSecretsManagerClientBuilder configureClient( | ||
| AWSSecretsManagerClientBuilder client); | ||
|
|
||
| @Override | ||
| protected boolean hasRequestId() { | ||
| return false; | ||
| } | ||
|
|
||
| @Test | ||
| public void sendCreateSecretRequestWithMockedResponse() throws Exception { | ||
| AWSSecretsManagerClientBuilder clientBuilder = AWSSecretsManagerClientBuilder.standard(); | ||
| AWSSecretsManager client = | ||
| configureClient(clientBuilder) | ||
| .withEndpointConfiguration(endpoint) | ||
| .withCredentials(credentialsProvider) | ||
| .build(); | ||
|
|
||
| String body = | ||
| "{" | ||
| + "\"ARN\": \"arn:aws:secretsmanager:us-west-2:123456789012:secret:MyTestDatabaseSecret-a1b2c3\"," | ||
| + "\"Name\": \"MyTestDatabaseSecret\"," | ||
| + "\"VersionId\": \"EXAMPLE1-90ab-cdef-fedc-ba987SECRET1\"" | ||
| + "}"; | ||
| server.enqueue(HttpResponse.of(HttpStatus.OK, MediaType.PLAIN_TEXT_UTF_8, body)); | ||
|
|
||
| Object response = | ||
| client.createSecret( | ||
| new CreateSecretRequest().withName("secretName").withSecretString("secretValue")); | ||
|
|
||
| List<AttributeAssertion> additionalAttributes = | ||
| singletonList( | ||
| equalTo( | ||
| AWS_SECRETSMANAGER_SECRET_ARN, | ||
| "arn:aws:secretsmanager:us-west-2:123456789012:secret:MyTestDatabaseSecret-a1b2c3")); | ||
|
|
||
| assertRequestWithMockedResponse( | ||
| response, client, "AWSSecretsManager", "CreateSecret", "POST", additionalAttributes); | ||
| } | ||
|
|
||
| @Test | ||
| public void sendDescribeSecretRequestWithMockedResponse() throws Exception { | ||
| AWSSecretsManagerClientBuilder clientBuilder = AWSSecretsManagerClientBuilder.standard(); | ||
| AWSSecretsManager client = | ||
| configureClient(clientBuilder) | ||
| .withEndpointConfiguration(endpoint) | ||
| .withCredentials(credentialsProvider) | ||
| .build(); | ||
|
|
||
| String body = | ||
| "{" | ||
| + "\"ARN\": \"arn:aws:secretsmanager:us-east-1:123456789012:secret:My-Secret-Id-WzAXar\"," | ||
| + "\"Name\": \"My-Secret-Id\"," | ||
| + "\"VersionId\": \"EXAMPLE1-90ab-cdef-fedc-ba987SECRET1\"" | ||
| + "}"; | ||
|
|
||
| server.enqueue(HttpResponse.of(HttpStatus.OK, MediaType.PLAIN_TEXT_UTF_8, body)); | ||
| Object response = | ||
| client.describeSecret(new DescribeSecretRequest().withSecretId("My-Secret-Id")); | ||
|
|
||
| List<AttributeAssertion> additionalAttributes = | ||
| singletonList( | ||
| equalTo( | ||
| AWS_SECRETSMANAGER_SECRET_ARN, | ||
| "arn:aws:secretsmanager:us-east-1:123456789012:secret:My-Secret-Id-WzAXar")); | ||
|
|
||
| assertRequestWithMockedResponse( | ||
| response, client, "AWSSecretsManager", "DescribeSecret", "POST", additionalAttributes); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this slightly magical this where when you run tests with
-PtestLatestDeps=truethe version for dependencies inlibraryscope (custom scope that iscompileOnly+testImplementation) is bumped to latest available version. This doesn't work with test suites, there we can do this manuallyUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know. I will update and run both tests.
This is addressed in the latest iteration.