feat(bottlecap): add aws trace header for java and sqs#452
Conversation
alexgallotta
commented
Nov 13, 2024
alexgallotta
commented
| 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 part.starts_with("Parent=") { | ||
| if parent_id.is_empty() { | ||
| parent_id = part[7..].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; | ||
| } |
There was a problem hiding this comment.
Can't we just do a simpler algo?
Root=1-64cc2edd-112fbf1701d1355973a11d57;Parent=7d5a9776024b2d42;Sampled=0
Split by ;
Root=1-64cc2edd-112fbf1701d1355973a11d57Parent=7d5a9776024b2d42Sampled=0
Then split by =?
There was a problem hiding this comment.
I chatgpt-translated the algorithm used in go.
I am not sure if there are side effects of using an easier split. I would argue a regexp would be much better and faster, but can you confirm that the format is a ; concatenations of those 3 headers?
There was a problem hiding this comment.
the logic seems to be to parse all the ; separated segments until all 3 variables are set. I did not want to spend too much time thinking on side effects
| ..Default::default() | ||
| }; | ||
|
|
||
| let mut is_step_function = false; |
There was a problem hiding this comment.
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.
Not sure what you mean
There was a problem hiding this comment.
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.
can you give me an example? I don't think it's doable or considered a good approach in rust
| // 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( |
There was a problem hiding this comment.
Why a crate method and not just a method for the struct? We are not using it anywhere else
There was a problem hiding this comment.
I prefer the flexibility and not adding extra restrictions that aren't needed. It makes testing and changing code simpler.
I also think a lot of the structs and traits should go away, so would rather avoid adding code and complexity to keep doing that
alexgallotta
force-pushed
the
alexgallotta/aws-trace-header
branch
from
3780a72 to
7128ed8
Compare
duncanista
force-pushed
the
jordan.gonzalez/bottlecap/universal-instrumentation
branch
from
dca2dd1 to
b52e738
Compare
alexgallotta
force-pushed
the
alexgallotta/aws-trace-header
branch
from
9f415b4 to
b188df1
Compare
alexgallotta
merged commit f2baa38
into
jordan.gonzalez/bottlecap/universal-instrumentation
* add aws trace header for java and sqs * fix priority sampling * remove clippy warnings * fix: do not skip inferred spans with aws headers * make clippy happy * add comment for 64 bits trace id * fix clippy warnings * fix import and tests * format * remove dead code
3 participants
