-
Notifications
You must be signed in to change notification settings - Fork 360
Add OpenTelemetry layer as optional #898
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
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 |
---|---|---|
|
@@ -2,7 +2,7 @@ use std::future::Future; | |
use std::pin::Pin; | ||
use std::task; | ||
|
||
use lambda_runtime::LambdaInvocation; | ||
use crate::LambdaInvocation; | ||
use opentelemetry_semantic_conventions::trace as traceconv; | ||
use pin_project::pin_project; | ||
use tower::{Layer, Service}; | ||
|
@@ -19,6 +19,7 @@ impl<F> OpenTelemetryLayer<F> | |
where | ||
F: Fn() + Clone, | ||
{ | ||
/// Create a new [OpenTelemetryLayer] with the provided flush function. | ||
pub fn new(flush_fn: F) -> Self { | ||
Self { flush_fn } | ||
} | ||
|
@@ -71,9 +72,14 @@ where | |
// After the first execution, we can set 'coldstart' to false | ||
self.coldstart = false; | ||
|
||
let fut = self.inner.call(req).instrument(span); | ||
let future = { | ||
// Enter the span before calling the inner service | ||
// to ensure that it's assigned as parent of the inner spans. | ||
let _guard = span.enter(); | ||
Comment on lines
+76
to
+78
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 didn't know that using And since the span is entered before starting to poll the future, it will increase the span duration, right? 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. See this PR: #896 |
||
self.inner.call(req) | ||
}; | ||
OpenTelemetryFuture { | ||
future: Some(fut), | ||
future: Some(future.instrument(span)), | ||
flush_fn: self.flush_fn.clone(), | ||
} | ||
} | ||
|
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.
line
67
:I think it should be configurable because the
lambda_runtime
crate can be used without using HTTP triggers (it could defaults tohttp
when using thelambda_http
crate though)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.
that sounds like a good idea. Feel free to open a PR with improvements
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.
👍 ➡️ #903