-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: add OTEL tracing span middleware during function proxy #1685
feat: add OTEL tracing span middleware during function proxy #1685
Conversation
@@ -154,6 +162,7 @@ func main() { | |||
scaler := scaling.NewFunctionScaler(scalingConfig, scalingFunctionCache) | |||
functionProxy = handlers.MakeScalingHandler(faasHandlers.Proxy, scaler, scalingConfig, config.Namespace) | |||
} | |||
functionProxy = tracing.Middleware(tracing.ConstantName("FunctionProxy"), functionProxy) |
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 applies the tracing middleware to only the function proxy, we chould also choose to add it to other endpoints though
} | ||
|
||
func debug(span trace.Span, format string, args ...interface{}) { | ||
value := os.Getenv("OTEL_LOG_LEVEL") |
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 standard env variable for the otel tooling log level, but it isn't fully respected by the go implementation yet. This was helpful for debugging some early issues with the middleware
gateway/pkg/tracing/otel.go
Outdated
// TracerProvider will also use a Resource configured with all the information | ||
// about the application. | ||
func Provider(ctx context.Context, name, version, commit string) (shutdown Shutdown, err error) { | ||
exporter := Exporter(os.Getenv("OTEL_EXPORTER")) |
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 not a standard env variable, we could choose a new name for it if you want.
gateway/pkg/tracing/otel.go
Outdated
case OTELExporter: | ||
// find available env variables for configuration | ||
// see: https://github.com/open-telemetry/opentelemetry-go/tree/main/exporters/otlp/otlptrace#environment-variables | ||
kind := get("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") |
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 also not standard, but i wanted a way to control which client the exporter used
efe658e
to
77a897a
Compare
Signed-off-by: Lucas Roesler <[email protected]>
77a897a
to
ea1a104
Compare
@alexellis one thing that I did not include in this is the automated scraping of node details when in EKS, i wasn't sure what if any vendors we wanted to support however, i do have an example here https://github.com/contiamo/go-base/blob/1dfd553de6d3b2da34a247991b609c67d7fc6659/pkg/otel/tracing/provider.go#L121-L132 of the changes that would be required to support EKS and integrating with x-ray |
Is there any progress in this task? thank you |
@berylshow it is currently stuck, @alexellis has asked the community for some input on use cases to gauge interest in the feature before we merge it. |
Description
Add tracing package the standardizes an OpenTelemetry tracing initialization and Middleware that can be applied to the Gateway handlers.
Add the tracing middleware to the function proxy handler only.
Requires new env variables to be enabled, it is otherwise a no-op.
Motivation and Context
design/approved
labelResolves #1684
How Has This Been Tested?
Manually tested by using the tracing walkthrough https://github.com/LucasRoesler/openfaas-tracing-walkthrough but modifying the Gateway image to a locally built image from this branch
Types of changes
Checklist:
git commit -s