-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add OTEL spans to SQS trigger #25
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>
@lann for some reason we're getting weird async Span issues and using |
@@ -125,7 +126,7 @@ impl TriggerExecutor for SqsTrigger { | |||
impl SqsTrigger { | |||
fn start_receive_loop(engine: Arc<TriggerAppEngine<Self>>, client: &aws::Client, component: &Component) -> tokio::task::JoinHandle<TerminationReason> { | |||
let future = Self::receive(engine, client.clone(), component.clone()); | |||
tokio::task::spawn(future) | |||
tokio::task::spawn(future.in_current_span()) |
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.
There is no current span at this point is there? From the docs that appears to make it equivalent to .instrument(Span::new_disabled(...))
.
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.
So it sounds like we may need to add a span to this function (#[instrument(name = "spin_trigger_sqs.start_receive_loop"
)? From our Spin triggers (HTTP and Redis), it seemed like an initial parent span was implied, so I am a little confused here
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.
This is the outer span for the HTTP trigger: https://github.com/fermyon/spin/blob/3c4d8320bacdf11c274c6feb91e6a7cd0dcfdaff/crates/trigger-http/src/lib.rs#L389
@@ -193,7 +194,7 @@ impl SqsMessageProcessor { | |||
queue_timeout_secs | |||
} | |||
} | |||
|
|||
#[instrument(name = "spin_trigger_sqs.process_message", skip_all, fields(otel.name = format!("{} receive", queue_name_from_url(&self.component.queue_url)), messaging.message.id = msg.display_id()))] |
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.
I guess I would still expect this span to show up correctly, but I don't really know how a disabled parent span would play into that...
TODO: Determine why span is only appearing periodically. Seems to do with how we are passing the context of the spans on
spawn
.