Conversation
| map<string, string> attributes = 3; | ||
|
|
||
| // This corresponds to Activity.Current?.Baggage | ||
| map<string, string> baggage = 4; |
There was a problem hiding this comment.
TODO: pull in final protobuf changes later
Here's the PR: Azure/azure-functions-language-worker-protobuf#99
There was a problem hiding this comment.
Pull request overview
Adds support for propagating distributed tracing baggage from the host to the .NET isolated worker so downstream telemetry/exporters can access it during an invocation (related to azure-functions-host#11026).
Changes:
- Extends the gRPC
RpcTraceContextcontract to includebaggage. - Plumbs received baggage from
InvocationRequest.TraceContextintoFunctionContext.Items. - Adds worker-side logic (core + OpenTelemetry middleware/extension) intended to apply the baggage to the ambient context for the invocation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/DotNetWorker.OpenTelemetry/OpenTelemetryWorkerBuilderExtensions.cs |
Adds a builder extension to enable baggage propagation via middleware. |
src/DotNetWorker.OpenTelemetry/BaggageMiddleware.cs |
Middleware that reads baggage from FunctionContext.Items and sets it into the ambient baggage context. |
src/DotNetWorker.Grpc/Handlers/InvocationHandler.cs |
Extracts baggage from the gRPC request trace context and stores it on context.Items. |
src/DotNetWorker.Core/Properties/AssemblyInfo.cs |
Grants DotNetWorker.OpenTelemetry access to Core internals (for TraceConstants). |
src/DotNetWorker.Core/FunctionsApplication.cs |
Adds logic to set baggage prior to invoking the function execution delegate. |
src/DotNetWorker.Core/Diagnostics/TraceConstants.cs |
Adds an internal key name for stashing baggage in FunctionContext.Items. |
protos/.../FunctionRpc.proto |
Adds baggage to RpcTraceContext as a map<string, string>. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
protos/azure-functions-language-worker-protobuf/src/proto/FunctionRpc.proto
Show resolved
Hide resolved
| { | ||
| public static class OpenTelemetryWorkerBuilderExtensions | ||
| { | ||
| public static IFunctionsWorkerApplicationBuilder EnableBaggagePropagation(this IFunctionsWorkerApplicationBuilder builder) |
There was a problem hiding this comment.
Is the expectation that customers call this manually? If so, there is going to be an ordering problem with it. Extension auto-registration will already have ran by the time a customer calls this, and so the baggage middleware will always be after any extension middleware. This means extension middleware misses out on all the baggage.
I think we will need a way to insert this at the start. Do we have middleware insert semantics? If no, we might need a way to supply this to the app builder so it gets registered before any extensions.
There was a problem hiding this comment.
Yes, this would be called manually in this design - let me see if there's anything existing that we can leverage to make sure this goes before extensions.
There was a problem hiding this comment.
I wonder if we need this to be a middleware approach at all. What if this sets a capability which we flow back to the host, and that turns host flowing baggage on/off. And then our implementation worker side can always populate baggage.
There was a problem hiding this comment.
What if we set this in FunctionsApplication.InvokeFunctionAsync method , before calling _functionExecutionDelegate? That way baggage is available to all middlewares. We are doing similar thing to setup Activity for the invocation there.
I like the capability idea @jviau suggested above. With this, we can send this capability with a new version of worker package (with an opt-out feature if necessary) and host can send this or not based on it. (Yea, this requires another host update, but will provide flexibility for workers to control this behavior).
Baggage comes from OTEL package and we can abstract this behavior (to set baggage) as a Feature and DotnetWorker.Otel project/package can implement the code (to set it if present in the invocation request)
There was a problem hiding this comment.
My first iteration used that approach (set baggage inside InvokeFunctionAsync) but it would require taking an otel dependency in core.
Moving the baggage propagation into middleware inside the otel extension was to avoid taking an otel dependency in core (not to control enablement/disablement). The host PR that was merged flows the baggage any time the conditions necessary are met (otel enabled, baggage exists) since we didn't have a strong case for disabling baggage (if those values are sent, why should they be blocked).
| { | ||
| public static class OpenTelemetryWorkerBuilderExtensions | ||
| { | ||
| public static IFunctionsWorkerApplicationBuilder EnableBaggagePropagation(this IFunctionsWorkerApplicationBuilder builder) |
There was a problem hiding this comment.
What if we set this in FunctionsApplication.InvokeFunctionAsync method , before calling _functionExecutionDelegate? That way baggage is available to all middlewares. We are doing similar thing to setup Activity for the invocation there.
I like the capability idea @jviau suggested above. With this, we can send this capability with a new version of worker package (with an opt-out feature if necessary) and host can send this or not based on it. (Yea, this requires another host update, but will provide flexibility for workers to control this behavior).
Baggage comes from OTEL package and we can abstract this behavior (to set baggage) as a Feature and DotnetWorker.Otel project/package can implement the code (to set it if present in the invocation request)
58b2ad0 to
ed4032e
Compare
Issue describing the changes in this PR
Related to the following issue: Azure/azure-functions-host#11026
Related host PR: Azure/azure-functions-host#11598
Pull request checklist
release_notes.md