Skip to content
Open
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 @@ -49,6 +49,8 @@ dependencies {
testImplementation group: 'com.amazonaws', name: 'aws-java-sdk-sqs', version: '[1.11,1.11.106]'
testImplementation group: 'com.amazonaws', name: 'aws-java-sdk-sns', version: '[1.11,1.11.106]'
testImplementation group: 'com.amazonaws', name: 'aws-java-sdk-dynamodb', version: '[1.11,1.11.106]'
testImplementation group: 'com.amazonaws', name: 'aws-java-sdk-stepfunctions', version: '[1.11,1.11.106]'
testImplementation group: 'com.amazonaws', name: 'aws-java-sdk-lambda', version: '[1.11,1.11.106]'

// needed for kinesis:
testImplementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: null
Expand Down Expand Up @@ -90,6 +92,13 @@ dependencies {
strictly '1.11.0'
}
}
// aws-java-sdk-stepfunctions (test dependency below 1.11.106) only exists from 1.11.63 and would
// otherwise pull a newer core than the 1.11.0 clients pinned above.
test_before_1_11_106Implementation(group: 'com.amazonaws', name: 'aws-java-sdk-core') {
version {
strictly '1.11.0'
}
}

dsmTestImplementation group: 'com.amazonaws', name: 'aws-java-sdk-kinesis', version: '1.12.366'
// no batch publish before v1.12
Expand All @@ -104,6 +113,8 @@ dependencies {
latestDepTestImplementation group: 'com.amazonaws', name: 'aws-java-sdk-sqs', version: '+'
latestDepTestImplementation group: 'com.amazonaws', name: 'aws-java-sdk-sns', version: '+'
latestDepTestImplementation group: 'com.amazonaws', name: 'aws-java-sdk-dynamodb', version: '+'
latestDepTestImplementation group: 'com.amazonaws', name: 'aws-java-sdk-stepfunctions', version: '+'
latestDepTestImplementation group: 'com.amazonaws', name: 'aws-java-sdk-lambda', version: '+'
}

tasks.named("test", Test) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ abstract class AWS1KinesisClientTest extends VersionedNamingTestBase {
"aws.agent" "java-aws-sdk"
"aws.stream.name" streamName
"streamname" streamName
"aws.stream.arn" streamArn
"$DDTags.PATHWAY_HASH" { String }
peerServiceFrom("aws.stream.name")
defaultTags()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ abstract class AWS1SnsClientTest extends VersionedNamingTestBase {
"aws.agent" "java-aws-sdk"
"aws.topic.name" topicName
"topicname" topicName
"aws.topic.arn" topicArn
"$DDTags.PATHWAY_HASH" { String }
peerServiceFrom("aws.topic.name")
defaultTags()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ protected void doOnRequest(final AgentSpan span, final Request request) {
String topicName = null;
String topicArn = access.getTopicArn(originalRequest);
if (null != topicArn) {
span.setTag(InstrumentationTags.AWS_TOPIC_ARN, topicArn);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Update the remaining strict AWS tag assertions

Requests in AWS1SnsClientTest, AWS1KinesisClientTest, and SnsClientTest in both SNS instrumentation modules now emit additional ARN tags, but their unchanged expectations omit them. The default tags assertion rejects every unverified tag, so these tests fail in assertTagsAllVerified(). Update those expectations for aws.topic.arn, aws.sns.topic_arn, and aws.stream.arn, alongside the SDK v2 expectations already updated in this PR.

Was this helpful? React 👍 or 👎
🤖 Bits Code Review · Open Bits AI session

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fixed. I updated the strict SDK v1 SNS and Kinesis DSM assertions and both SNS instrumentation-module test suites. I also reran all 17 affected minimum, latest-dependency, forked, and DSM test tasks successfully.

topicName = topicArn.substring(topicArn.lastIndexOf(':') + 1);
span.setTag(InstrumentationTags.AWS_TOPIC_NAME, topicName);
span.setTag(InstrumentationTags.TOPIC_NAME, topicName);
Expand All @@ -169,6 +170,7 @@ protected void doOnRequest(final AgentSpan span, final Request request) {
}
String streamArn = access.getStreamARN(originalRequest);
if (null != streamArn) {
span.setTag(InstrumentationTags.AWS_STREAM_ARN, streamArn);
int streamNameStart = streamArn.indexOf(":stream/");
if (streamNameStart >= 0) {
streamName = streamArn.substring(streamNameStart + 8);
Expand All @@ -185,6 +187,20 @@ protected void doOnRequest(final AgentSpan span, final Request request) {
bestPrecursor = InstrumentationTags.AWS_TABLE_NAME;
bestPeerService = tableName;
}
String stateMachineArn = access.getStateMachineArn(originalRequest);
if (null != stateMachineArn) {
span.setTag(InstrumentationTags.AWS_STATE_MACHINE_ARN, stateMachineArn);
}
String executionArn = access.getExecutionArn(originalRequest);
if (null != executionArn) {
span.setTag(InstrumentationTags.AWS_EXECUTION_ARN, executionArn);
}
if ("AWSLambda".equalsIgnoreCase(awsServiceName)) {
String functionName = access.getFunctionName(originalRequest);
if (null != functionName) {
setFunctionIdentity(span, functionName);
}
}

// Set peer.service based on Config for serverless functions
if (Config.get().isAwsServerless()) {
Expand Down Expand Up @@ -248,6 +264,39 @@ protected void doOnRequest(final AgentSpan span, final Request request) {
}
}

/**
* Normalizes a Lambda {@code FunctionName} request value. {@code aws.function.name} is always the
* bare function name (account, {@code function:} prefix and {@code :qualifier} suffix stripped),
* and {@code aws.function.arn} is set only when the value is a full ARN. A partial ARN ({@code
* account:function:name}) yields the name but no ARN, because its Region and partition are
* unknown and must not be invented.
*/
private static void setFunctionIdentity(AgentSpan span, String functionName) {
if (functionName == null || functionName.isEmpty()) {
return;
}
if (functionName.startsWith("arn:")) {
// arn:<partition>:lambda:<region>:<account>:function:<name>[:<qualifier>]
span.setTag(InstrumentationTags.AWS_FUNCTION_ARN, functionName);
}
span.setTag(InstrumentationTags.AWS_FUNCTION_NAME, lambdaFunctionName(functionName));
}

/** Extracts the bare function name from any accepted {@code FunctionName} form. */
static String lambdaFunctionName(String functionName) {
String[] parts = functionName.split(":");
// Full ARN: arn:<partition>:lambda:<region>:<account>:function:<name>[:<qualifier>]
if (parts.length >= 7 && "arn".equals(parts[0]) && "function".equals(parts[5])) {
return parts[6];
}
// Partial ARN: <account>:function:<name>[:<qualifier>]
if (parts.length >= 3 && "function".equals(parts[1])) {
return parts[2];
}
// Bare name, optionally with a :alias or :version qualifier (parts[0] is the name).
return parts[0];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Guard empty splits in both Lambda name parsers

For a Lambda FunctionName such as ":" or "::", split(":") returns an empty array and parts[0] throws ArrayIndexOutOfBoundsException. Instead of allowing normal SDK/service validation, tracing aborts the request. In SDK v1, the exception occurs before the new span is stored in handler context, leaving it unfinished. The SDK v2 parser has the same unchecked access; guard empty split results in both implementations.

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

}

public void onServiceResponse(
final AgentSpan span, final String awsService, final Response response) {
if ("s3".equalsIgnoreCase(simplifyServiceName(awsService))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ static GetterAccess of(final Object request) {
private final MethodHandle getPublishBatchRequestEntries;
private final MethodHandle getApproximateArrivalTimestamp;
private final MethodHandle getTableName;
private final MethodHandle getStateMachineArn;
private final MethodHandle getExecutionArn;
private final MethodHandle getFunctionName;

private GetterAccess(final Class<?> objectType) {
operationName =
Expand All @@ -55,6 +58,9 @@ private GetterAccess(final Class<?> objectType) {
getApproximateArrivalTimestamp =
findGetter(objectType, "getApproximateArrivalTimestamp", Date.class);
getTableName = findStringGetter(objectType, "getTableName");
getStateMachineArn = findStringGetter(objectType, "getStateMachineArn");
getExecutionArn = findStringGetter(objectType, "getExecutionArn");
getFunctionName = findStringGetter(objectType, "getFunctionName");
}

String getOperationNameFromType() {
Expand Down Expand Up @@ -101,6 +107,18 @@ String getTableName(final Object object) {
return invokeForString(getTableName, object);
}

String getStateMachineArn(final Object object) {
return invokeForString(getStateMachineArn, object);
}

String getExecutionArn(final Object object) {
return invokeForString(getExecutionArn, object);
}

String getFunctionName(final Object object) {
return invokeForString(getFunctionName, object);
}

Date getApproximateArrivalTimestamp(final Object object) {
return invoke(getApproximateArrivalTimestamp, object);
}
Expand Down
Loading
Loading