Skip to content

Capture S3 bucket+key as otel attributes #3136

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
May 5, 2023
Merged

Capture S3 bucket+key as otel attributes #3136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
769ff80
refactor tests a bit with common span assertions
SylvainJuge May 3, 2023
fa1d7c4
enhance ability to provide otel attributes
SylvainJuge May 4, 2023
4b43892
implement s3 otel attributes for sdk v1
SylvainJuge May 4, 2023
298766f
implement s3 for sdk v2 + improve tests
SylvainJuge May 4, 2023
e0d4b8a
migrate dynamodb tests
SylvainJuge May 4, 2023
4b175f7
migrate sqs tests
SylvainJuge May 4, 2023
2a5fcd8
remove old test methods
SylvainJuge May 4, 2023
c1e7118
remove todo for not-normalized SQS action name
SylvainJuge May 4, 2023
76efee6
Merge branch 'main' of github.com:elastic/apm-agent-java into s3-buck…
SylvainJuge May 4, 2023
e39e8c7
update changelog
SylvainJuge May 4, 2023
aaef646
remove useless duplication
SylvainJuge May 5, 2023
736b81c
fix compilation error
SylvainJuge May 5, 2023
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ endif::[]

=== Unreleased

[[release-notes-1.39.0]]
==== 1.39.0 - YYYY/MM/DD

[float]
===== Features
* Capture S3 operation details as OTel attributes - {pull}3136[#3136]

[[release-notes-1.x]]
=== Java Agent version 1.x

Expand Down
10 changes: 10 additions & 0 deletions apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/AbstractSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ public T withSync(boolean sync) {
return thiz();
}



@Nullable
protected static String normalizeEmpty(@Nullable String value) {
return value == null || value.isEmpty() ? null : value;
Expand Down Expand Up @@ -787,6 +789,14 @@ public Map<String, Object> getOtelAttributes() {
return otelAttributes;
}

@Override
public T withOtelAttribute(String key, @Nullable Object value) {
if (value != null) {
otelAttributes.put(key, value);
}
return thiz();
}

@Override
@Nullable
public String getType() {
Expand Down
8 changes: 4 additions & 4 deletions ...agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1519,10 +1519,10 @@ private <T extends AbstractSpan<T>> void testOTelSpanSerialization(T context, Fu
}

// with custom otel attributes
context.getOtelAttributes().put("attribute.string", "hello");
context.getOtelAttributes().put("attribute.long", 123L);
context.getOtelAttributes().put("attribute.boolean", false);
context.getOtelAttributes().put("attribute.float", 0.42f);
context.withOtelAttribute("attribute.string", "hello");
context.withOtelAttribute("attribute.long", 123L);
context.withOtelAttribute("attribute.boolean", false);
context.withOtelAttribute("attribute.float", 0.42f);
spanJson = toJson.apply(context);
JsonNode otelJson = spanJson.get("otel");
assertThat(otelJson).isNotNull();
Expand Down
12 changes: 12 additions & 0 deletions ...gent-core/src/test/java/co/elastic/apm/agent/testutils/assertions/AbstractSpanAssert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import co.elastic.apm.agent.impl.transaction.AbstractSpan;
import co.elastic.apm.agent.impl.transaction.TraceContext;
import co.elastic.apm.agent.tracer.Outcome;

import java.util.Objects;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -89,6 +90,17 @@ public SELF hasSpanLink(AbstractSpan<?> expectedLink) {
return thiz();
}

public SELF hasOutcome(Outcome expectedOutcome) {
checkObject("Expected span to have outcome %, but was %s", expectedOutcome, actual.getOutcome());
return thiz();
}

public SELF hasOtelAttribute(String key, Object expectedValue) {
isNotNull();
checkObject("Expected span to have value '%s' for attribute '" + key + "' but was '%s' ",expectedValue, actual.getOtelAttributes().get(key));
return thiz();
}

private boolean checkSpanLinksContain(TraceContext expectedLink) {
return actual.getSpanLinks().stream()
.anyMatch(ctx -> Objects.equals(ctx.getParentId(), expectedLink.getId())
Expand Down
34 changes: 34 additions & 0 deletions ...dk/apm-aws-sdk-1-plugin/src/main/java/co/elastic/apm/agent/awssdk/v1/helper/S3Helper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import com.amazonaws.Request;
import com.amazonaws.http.ExecutionContext;

import javax.annotation.Nullable;

public class S3Helper extends AbstractS3InstrumentationHelper<Request<?>, ExecutionContext> {

private static final S3Helper INSTANCE = new S3Helper(GlobalTracer.get());
Expand All @@ -35,4 +37,36 @@ public static S3Helper getInstance() {
public S3Helper(Tracer tracer) {
super(tracer, SdkV1DataSource.getInstance());
}

@Nullable
@Override
protected String getObjectKey(Request<?> request, @Nullable String bucketName) {
Copy link
Member Author

@SylvainJuge SylvainJuge May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[for reviewer] with SDK v1 we have to use some manual parsing of an HTTP header, this is not the case with v2.
if (bucketName == null) {
return null;
}
String resourcePath = request.getResourcePath();
if (!resourcePath.startsWith(bucketName)) {
return null;
}
int start = bucketName.length();
if (start < resourcePath.length() && resourcePath.charAt(start) == '/') {
start++;
}
int end = resourcePath.length();
if (start < end) {
return resourcePath.substring(start, end);
}
return null;
}

@Nullable
@Override
protected String getCopySource(Request<?> request) {
String header = request.getHeaders().get("x-amz-copy-source");
if (header != null && header.startsWith("/")) {
header = header.substring(1);
}
return header;

}
}
163 changes: 104 additions & 59 deletions ...k/apm-aws-sdk-1-plugin/src/test/java/co/elastic/apm/agent/awssdk/v1/DynamoDbClientIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package co.elastic.apm.agent.awssdk.v1;

import co.elastic.apm.agent.awssdk.common.AbstractAwsClientIT;
import co.elastic.apm.agent.impl.transaction.AbstractSpan;
import co.elastic.apm.agent.impl.transaction.Span;
import co.elastic.apm.agent.impl.transaction.Transaction;
import com.amazonaws.auth.AWSStaticCredentialsProvider;
Expand Down Expand Up @@ -75,40 +74,63 @@ public void setupClient() {
public void testDynamoDbClient() {
Transaction transaction = startTestRootTransaction("s3-test");

executeTest("CreateTable", "query", TABLE_NAME, () -> dynamoDB.createTable(new CreateTableRequest().withTableName(TABLE_NAME)
.withAttributeDefinitions(List.of(
new AttributeDefinition("attributeOne", ScalarAttributeType.S),
new AttributeDefinition("attributeTwo", ScalarAttributeType.N)
))
.withKeySchema(List.of(
new KeySchemaElement("attributeOne", KeyType.HASH),
new KeySchemaElement("attributeTwo", KeyType.RANGE)
))
.withBillingMode(BillingMode.PAY_PER_REQUEST)),
dbAssert);

executeTest("ListTables", "query", null, () -> dynamoDB.listTables(),
dbAssert);

executeTest("PutItem", "query", TABLE_NAME, () -> dynamoDB.putItem(
newTest(() -> dynamoDB.createTable(new CreateTableRequest().withTableName(TABLE_NAME)
.withAttributeDefinitions(List.of(
new AttributeDefinition("attributeOne", ScalarAttributeType.S),
new AttributeDefinition("attributeTwo", ScalarAttributeType.N)
))
.withKeySchema(List.of(
new KeySchemaElement("attributeOne", KeyType.HASH),
new KeySchemaElement("attributeTwo", KeyType.RANGE)
))
.withBillingMode(BillingMode.PAY_PER_REQUEST)))
.operationName("CreateTable")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert)
.execute();
Comment on lines +77 to +91
Copy link
Member Author

@SylvainJuge SylvainJuge May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[for reviewer] this is mostly automatic refactoring with a "test builder" that allows to combine assertions on the spans in a more declarative way.

newTest(() -> dynamoDB.listTables())
.operationName("ListTables")
.action("query")
.withSpanAssertions(dbAssert)
.execute();

newTest(() -> dynamoDB.putItem(
new PutItemRequest(TABLE_NAME,
Map.of("attributeOne", new AttributeValue("valueOne"), "attributeTwo", new AttributeValue().withN("10")))),
dbAssert);

executeTest("Query", "query", TABLE_NAME, () -> dynamoDB.query(
Map.of("attributeOne", new AttributeValue("valueOne"), "attributeTwo", new AttributeValue().withN("10")))))
.operationName("PutItem")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert)
.execute();

newTest(() -> dynamoDB.query(
new QueryRequest(TABLE_NAME)
.withKeyConditionExpression(KEY_CONDITION_EXPRESSION)
.withExpressionAttributeValues(Map.of(":one", new AttributeValue("valueOne")))),
dbAssert
.andThen(span -> assertThat(span.getContext().getDb()).hasStatement(KEY_CONDITION_EXPRESSION)));

executeTest("DeleteTable", "query", TABLE_NAME, () -> dynamoDB.deleteTable(TABLE_NAME),
dbAssert);

executeTestWithException(ResourceNotFoundException.class, "PutItem", "query", TABLE_NAME + "-exception", () -> dynamoDB.putItem(
.withExpressionAttributeValues(Map.of(":one", new AttributeValue("valueOne")))))
.operationName("Query")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert
.andThen(span -> assertThat(span.getContext().getDb()).hasStatement(KEY_CONDITION_EXPRESSION)))
.execute();

newTest(() -> dynamoDB.deleteTable(TABLE_NAME))
.operationName("DeleteTable")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert)
.execute();

newTest(() -> dynamoDB.putItem(
new PutItemRequest(TABLE_NAME + "-exception",
Map.of("attributeOne", new AttributeValue("valueOne"), "attributeTwo", new AttributeValue().withN("10")))),
dbAssert);
Map.of("attributeOne", new AttributeValue("valueOne"), "attributeTwo", new AttributeValue().withN("10")))))
.operationName("PutItem")
.action("query")
.entityName(TABLE_NAME + "-exception")
.withSpanAssertions(dbAssert)
.executeWithException(ResourceNotFoundException.class);

assertThat(reporter.getSpans().size()).isEqualTo(6);

Expand All @@ -122,41 +144,64 @@ public void testDynamoDbClient() {
public void testDynamoDbClientAsync() {
Transaction transaction = startTestRootTransaction("s3-test");

executeTest("CreateTable", "query", TABLE_NAME, () -> dynamoDBAsync.createTableAsync(new CreateTableRequest().withTableName(TABLE_NAME)
.withAttributeDefinitions(List.of(
new AttributeDefinition("attributeOne", ScalarAttributeType.S),
new AttributeDefinition("attributeTwo", ScalarAttributeType.N)
))
.withKeySchema(List.of(
new KeySchemaElement("attributeOne", KeyType.HASH),
new KeySchemaElement("attributeTwo", KeyType.RANGE)
))
.withBillingMode(BillingMode.PAY_PER_REQUEST)),
dbAssert);

executeTest("ListTables", "query", null, () -> dynamoDBAsync.listTablesAsync(),
dbAssert);

executeTest("PutItem", "query", TABLE_NAME, () -> dynamoDBAsync.putItemAsync(
newTest(() -> dynamoDBAsync.createTableAsync(new CreateTableRequest().withTableName(TABLE_NAME)
.withAttributeDefinitions(List.of(
new AttributeDefinition("attributeOne", ScalarAttributeType.S),
new AttributeDefinition("attributeTwo", ScalarAttributeType.N)
))
.withKeySchema(List.of(
new KeySchemaElement("attributeOne", KeyType.HASH),
new KeySchemaElement("attributeTwo", KeyType.RANGE)
))
.withBillingMode(BillingMode.PAY_PER_REQUEST)))
.operationName("CreateTable")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert)
.async()
.execute();

newTest(() -> dynamoDBAsync.listTablesAsync())
.operationName("ListTables")
.action("query")
.withSpanAssertions(dbAssert)
.async()
.execute();

newTest(() -> dynamoDBAsync.putItemAsync(
new PutItemRequest(TABLE_NAME,
Map.of("attributeOne", new AttributeValue("valueOne"), "attributeTwo", new AttributeValue().withN("10")))),
dbAssert);

executeTest("Query", "query", TABLE_NAME, () -> dynamoDBAsync.queryAsync(
Map.of("attributeOne", new AttributeValue("valueOne"), "attributeTwo", new AttributeValue().withN("10")))))
.operationName("PutItem")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert)
.async()
.execute();

newTest(() -> dynamoDBAsync.queryAsync(
new QueryRequest(TABLE_NAME)
.withKeyConditionExpression(KEY_CONDITION_EXPRESSION)
.withExpressionAttributeValues(Map.of(":one", new AttributeValue("valueOne")))),
dbAssert
.andThen(span -> assertThat(span.getContext().getDb()).hasStatement(KEY_CONDITION_EXPRESSION)));

executeTest("DeleteTable", "query", TABLE_NAME, () -> dynamoDBAsync.deleteTableAsync(TABLE_NAME),
dbAssert);
.withExpressionAttributeValues(Map.of(":one", new AttributeValue("valueOne")))))
.operationName("Query")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert
.andThen(span ->
assertThat(span.getContext().getDb()).hasStatement(KEY_CONDITION_EXPRESSION)))
.async()
.execute();

newTest(() -> dynamoDBAsync.deleteTableAsync(TABLE_NAME))
.operationName("DeleteTable")
.entityName(TABLE_NAME)
.action("query")
.withSpanAssertions(dbAssert)
.async()
.execute();

assertThat(reporter.getSpans().size()).isEqualTo(5);

transaction.deactivate().end();

assertThat(reporter.getSpans()).noneMatch(AbstractSpan::isSync);
}

@Override
Expand Down
Loading