-
Notifications
You must be signed in to change notification settings - Fork 20
feat(bottlecap): add aws trace header for java and sqs #452
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 all commits
7213036
5a69969
6424aaf
e8a101f
2d28348
e1bf0bc
cda5268
73d851c
f798b84
b188df1
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 |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ use crate::lifecycle::invocation::{ | |
| ServiceNameResolver, Trigger, DATADOG_CARRIER_KEY, FUNCTION_TRIGGER_EVENT_SOURCE_TAG, | ||
| }, | ||
| }; | ||
| use crate::traces::context::{Sampling, SpanContext}; | ||
|
|
||
| #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] | ||
| pub struct SqsEvent { | ||
|
|
@@ -64,6 +65,8 @@ pub struct Attributes { | |
| pub sent_timestamp: String, | ||
| #[serde(rename = "SenderId")] | ||
| pub sender_id: String, | ||
| #[serde(rename = "AWSTraceHeader")] | ||
| pub aws_trace_header: Option<String>, | ||
| } | ||
|
|
||
| impl Trigger for SqsRecord { | ||
|
|
@@ -163,12 +166,9 @@ impl Trigger for SqsRecord { | |
| } | ||
| } | ||
|
|
||
| fn is_async(&self) -> bool { | ||
| true | ||
| } | ||
|
|
||
| fn get_carrier(&self) -> HashMap<String, String> { | ||
| let carrier = HashMap::new(); | ||
|
|
||
| if let Some(ma) = self.message_attributes.get(DATADOG_CARRIER_KEY) { | ||
| if let Some(string_value) = &ma.string_value { | ||
| return serde_json::from_str(string_value).unwrap_or_default(); | ||
|
|
@@ -190,6 +190,10 @@ impl Trigger for SqsRecord { | |
| // TODO: AWSTraceHeader | ||
| carrier | ||
| } | ||
|
|
||
| fn is_async(&self) -> bool { | ||
| true | ||
| } | ||
| } | ||
|
|
||
| impl ServiceNameResolver for SqsRecord { | ||
|
|
@@ -206,7 +210,74 @@ impl ServiceNameResolver for SqsRecord { | |
| } | ||
| } | ||
|
|
||
| // extractTraceContextfromAWSTraceHeader extracts trace context from the | ||
| // AWSTraceHeader directly. Unlike the other carriers in this file, it should | ||
| // not be passed to the tracer.Propagator, instead extracting context directly. | ||
| pub(crate) fn extract_trace_context_from_aws_trace_header( | ||
|
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. Why a crate method and not just a method for the struct? We are not using it anywhere else
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. I prefer the flexibility and not adding extra restrictions that aren't needed. It makes testing and changing code simpler. |
||
| headers_string: Option<String>, | ||
| ) -> Option<SpanContext> { | ||
| let value = headers_string?; | ||
| if !value.starts_with("Root=") { | ||
| return None; | ||
| } | ||
|
|
||
| let mut start_part = 0; | ||
| let mut trace_id = String::new(); | ||
| let mut parent_id = String::new(); | ||
| let mut sampled = String::new(); | ||
|
|
||
| let length = value.len(); | ||
| while start_part < length { | ||
| let end_part = value[start_part..] | ||
| .find(';') | ||
| .map_or(length, |i| i + start_part); | ||
| let part = &value[start_part..end_part]; | ||
|
|
||
| if part.starts_with("Root=") { | ||
| if trace_id.is_empty() { | ||
| trace_id = part[24..].to_string(); | ||
| } | ||
| } else if let Some(parent_part) = part.strip_prefix("Parent=") { | ||
| if parent_id.is_empty() { | ||
| parent_id = parent_part.to_string(); | ||
| } | ||
| } else if part.starts_with("Sampled=") && sampled.is_empty() { | ||
| sampled = part[8..].to_string(); | ||
| } | ||
|
|
||
| if !trace_id.is_empty() && !parent_id.is_empty() && !sampled.is_empty() { | ||
| break; | ||
| } | ||
| start_part = end_part + 1; | ||
| } | ||
|
|
||
| let trace_id = u64::from_str_radix(&trace_id, 16).ok()?; | ||
| let parent_id = u64::from_str_radix(&parent_id, 16).ok()?; | ||
|
|
||
| if trace_id == 0 || parent_id == 0 { | ||
| debug!("awstrace_header contains empty trace or parent ID"); | ||
| return None; | ||
| } | ||
|
|
||
| let sampling_priority = i8::from(sampled == "1"); | ||
|
|
||
| Some(SpanContext { | ||
| // the context from AWS Header is used by Datadog only and does not contain the upper | ||
| // 64 bits like other 128 w3c compliant trace ids | ||
| trace_id, | ||
| span_id: parent_id, | ||
| sampling: Some(Sampling { | ||
| priority: Some(sampling_priority), | ||
| mechanism: None, | ||
| }), | ||
| origin: None, | ||
| tags: HashMap::new(), | ||
|
alexgallotta marked this conversation as resolved.
|
||
| links: Vec::new(), | ||
| }) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| #[allow(clippy::unwrap_used)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::lifecycle::invocation::triggers::test_utils::read_json_file; | ||
|
|
@@ -235,6 +306,7 @@ mod tests { | |
| approximate_receive_count: "1".to_string(), | ||
| sent_timestamp: "1523232000000".to_string(), | ||
| sender_id: "123456789012".to_string(), | ||
| aws_trace_header: None, | ||
| }, | ||
| message_attributes, | ||
| md5_of_body: "{{{md5_of_body}}}".to_string(), | ||
|
|
@@ -425,4 +497,29 @@ mod tests { | |
| "generic-service" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn extract_java_sqs_header_context() { | ||
| let json = read_json_file("eventbridge_sqs_java_header_event.json"); | ||
| let payload = serde_json::from_str(&json).expect("Failed to deserialize into Value"); | ||
| let event = SqsRecord::new(payload).expect("Failed to deserialize EventBridgeEvent"); | ||
|
|
||
| assert_eq!( | ||
| extract_trace_context_from_aws_trace_header(Some( | ||
| event.attributes.aws_trace_header.unwrap().to_string() | ||
| )) | ||
| .unwrap(), | ||
| SpanContext { | ||
| trace_id: 130_944_522_478_755_159, | ||
| span_id: 9_032_698_535_745_367_362, | ||
| sampling: Some(Sampling { | ||
| priority: Some("0".parse().unwrap()), | ||
| mechanism: None, | ||
| }), | ||
| origin: None, | ||
| tags: HashMap::new(), | ||
| links: Vec::new(), | ||
| } | ||
| ); | ||
| } | ||
| } | ||
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.
Can't we just downcast into the StepFunctions event instead of having a boolean? That way at the very bottom you can just check if the trigger is StepFunction
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.
Not sure what you mean
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.
instead of having a boolean, where you have this thing, just try downcasting the boxed trigger into a stepfunctions event, if it succeeds, then its guaranteed to be a stepfunction event
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.
can you give me an example? I don't think it's doable or considered a good approach in rust