Skip to content

DynamoDB Span Pointers#619

Merged
nhulston merged 14 commits intomainfrom
nicholas.hulston/dynamodb-span-pointers
Mar 25, 2025
Merged

DynamoDB Span Pointers#619
nhulston merged 14 commits intomainfrom
nicholas.hulston/dynamodb-span-pointers

Conversation

@nhulston
Copy link
Copy Markdown
Contributor

@nhulston nhulston commented Mar 24, 2025

What does this PR do?

Adds span pointers to spans for Lambdas triggered by DynamoDB streams. This change will affect the downstream case for Universal Instrumentation Lambda runtimes (Java, .NET, Golang).

Span pointers are similar to Span Links, but for cases when it is impossible to pass the Trace ID and Span ID between the spans that need to be linked.

When the calculated hashes for the upstream and downstream lambdas match, the Datadog frontend will automatically link the two traces together.

Upstream case (requires tracer to be instrumented):
Screenshot 2025-03-24 at 1 46 12 PM

Downstream case (this code):
Screenshot 2025-03-24 at 1 46 56 PM

When clicking on the linked span, a new tab opens linking to the opposite Lambda function.

Describe how you validated your changes

Mostly manual testing, but I also added unit tests. There are also functional tests in https://github.com/DataDog/serverless-e2e-tests -- DynamoDB is TODO but will be implemented soon.

@nhulston nhulston marked this pull request as ready for review March 24, 2025 18:50
@nhulston nhulston requested a review from a team as a code owner March 24, 2025 18:50
Comment on lines +183 to +207
let (primary_key1, value1, primary_key2, value2) = if self.dynamodb.keys.len() == 1 {
// Single primary key case
let (key, attr_value) = self
.dynamodb
.keys
.iter()
.next()
.expect("No DynamoDB keys found");

let value = attr_value.to_string()?;
(key.clone(), value, String::new(), String::new())
} else {
// Two keys case, sort lexicographically for consistent ordering
let mut keys: Vec<(&String, &AttributeValue)> = self.dynamodb.keys.iter().collect();
keys.sort_by(|a, b| a.0.cmp(b.0));

let (k1, attr1) = keys[0];
// If unable to get string value, just return None
let v1 = attr1.to_string()?;

let (k2, attr2) = keys[1];
let v2 = attr2.to_string()?;

(k1.clone(), v1, k2.clone(), v2)
};
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.

Suggested change
let (primary_key1, value1, primary_key2, value2) = if self.dynamodb.keys.len() == 1 {
// Single primary key case
let (key, attr_value) = self
.dynamodb
.keys
.iter()
.next()
.expect("No DynamoDB keys found");
let value = attr_value.to_string()?;
(key.clone(), value, String::new(), String::new())
} else {
// Two keys case, sort lexicographically for consistent ordering
let mut keys: Vec<(&String, &AttributeValue)> = self.dynamodb.keys.iter().collect();
keys.sort_by(|a, b| a.0.cmp(b.0));
let (k1, attr1) = keys[0];
// If unable to get string value, just return None
let v1 = attr1.to_string()?;
let (k2, attr2) = keys[1];
let v2 = attr2.to_string()?;
(k1.clone(), v1, k2.clone(), v2)
};
let (primary_key1, value1, primary_key2, value2) = match self.dynamodb.keys.len() {
1 => {
let (key, attr_value) = self.dynamodb.keys.iter()
.next()
.expect("No DynamoDB keys found");
(key.clone(), attr_value.to_string()?, String::new(), String::new())
},
_ => {
let mut sorted_keys: Vec<_> = self.dynamodb.keys.iter()
.collect::<Vec<_>>()
.into_iter()
.sorted_by_key(|k| k.0.clone())
.collect();
let (k1, attr1) = sorted_keys[0];
let (k2, attr2) = sorted_keys[1];
(k1.clone(), attr1.to_string()?, k2.clone(), attr2.to_string()?)
}
};

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.

Ideally, I'd not even return anything, but maybe push to the array that you're creating? Although if you push and someone modifies the code, they might break the order (in case the order matters), anyhow, looks good if you don't decide to change the return thing,

Nonetheless, I'd encourage the suggestion above

Copy link
Copy Markdown
Contributor

@duncanista duncanista left a comment

Choose a reason for hiding this comment

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

LGTM – left a comment

@nhulston nhulston merged commit 293c624 into main Mar 25, 2025
45 checks passed
@nhulston nhulston deleted the nicholas.hulston/dynamodb-span-pointers branch March 25, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants