Skip to content
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

Fix context leak on call to AmazonS3.generatePresignedUrl #8815

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ testing {
implementation("com.amazonaws:aws-java-sdk-kinesis:1.11.0")
implementation("com.amazonaws:aws-java-sdk-dynamodb:1.11.0")
implementation("com.amazonaws:aws-java-sdk-sns:1.11.0")

// needed by S3
implementation("javax.xml.bind:jaxb-api:2.3.1")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
package io.opentelemetry.javaagent.instrumentation.awssdk.v1_11

import com.amazonaws.AmazonWebServiceClient
import com.amazonaws.ClientConfiguration
import com.amazonaws.Request
import com.amazonaws.auth.BasicAWSCredentials
import com.amazonaws.auth.NoOpSigner
import com.amazonaws.auth.SignerFactory
import com.amazonaws.handlers.RequestHandler2
import com.amazonaws.regions.Regions
import com.amazonaws.services.s3.AmazonS3Client
Expand Down Expand Up @@ -110,4 +113,20 @@ class Aws1ClientTest extends AbstractAws1ClientTest implements AgentTestTrait {
}
}
}

def "calling generatePresignedUrl does not leak context"() {
setup:
SignerFactory.registerSigner("noop", NoOpSigner)
def client = AmazonS3ClientBuilder.standard()
.withRegion(Regions.US_EAST_1)
.withClientConfiguration(new ClientConfiguration().withSignerOverride("noop"))
.build()

when:
client.generatePresignedUrl("someBucket", "someKey", new Date())

then:
// expecting no active span after call to generatePresignedUrl
!Span.current().getSpanContext().isValid()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import com.amazonaws.auth.AWSCredentialsProviderChain
import com.amazonaws.auth.BasicAWSCredentials
import com.amazonaws.auth.EnvironmentVariableCredentialsProvider
import com.amazonaws.auth.InstanceProfileCredentialsProvider
import com.amazonaws.auth.NoOpSigner
import com.amazonaws.auth.SignerFactory
import com.amazonaws.auth.SystemPropertiesCredentialsProvider
import com.amazonaws.auth.profile.ProfileCredentialsProvider
import com.amazonaws.handlers.RequestHandler2
Expand Down Expand Up @@ -272,4 +274,18 @@ class Aws0ClientTest extends AgentInstrumentationSpecification {
}
}
}

def "calling generatePresignedUrl does not leak context"() {
setup:
SignerFactory.registerSigner("noop", NoOpSigner)
def client = new AmazonS3Client(new ClientConfiguration().withSignerOverride("noop"))
.withEndpoint("${server.httpUri()}")

when:
client.generatePresignedUrl("someBucket", "someKey", new Date())

then:
// expecting no active span after call to generatePresignedUrl
!Span.current().getSpanContext().isValid()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ final class TracingRequestHandler extends RequestHandler2 {
@Override
@SuppressWarnings("deprecation") // deprecated class to be updated once published in new location
public void beforeRequest(Request<?> request) {
// GeneratePresignedUrlRequest doesn't result in actual request, beforeRequest is the only
// method called for it. Span created here would never be ended and scope would be leaked when
// running with java agent.
if ("com.amazonaws.services.s3.model.GeneratePresignedUrlRequest"
.equals(request.getOriginalRequest().getClass().getName())) {
return;
}

Context parentContext = Context.current();
if (!requestInstrumenter.shouldStart(parentContext, request)) {
return;
Expand Down