Lazy HttpTrigger binding data #675
Conversation
| @@ -19,7 +19,7 @@ | |||
| <ItemGroup> | |||
| <PackageReference Include="Microsoft.Azure.DocumentDB.ChangeFeedProcessor" Version="2.3.0" /> | |||
| <PackageReference Include="Microsoft.Azure.DocumentDB.Core" Version="2.10.3" /> | |||
| <PackageReference Include="Microsoft.Azure.WebJobs" Version="3.0.16" /> | |||
| <PackageReference Include="Microsoft.Azure.WebJobs" Version="3.0.22" /> | |||
There was a problem hiding this comment.
Just noticed that these were all quite behind.
| // apply some invocation details to the request that can be used later | ||
| // in the http request pipeline | ||
| request.HttpContext.Items[ScopeKeys.FunctionInvocationId] = context.FunctionInstanceId.ToString(); | ||
| request.HttpContext.Items[ScopeKeys.FunctionName] = context.FunctionContext.MethodName; |
There was a problem hiding this comment.
I'm going to use these properties to allow me to remove 2 of the 4 ETW invocation events we emit on each http function invocation. For http, we have trace middleware in the pipeline that writes detailed logs for all http requests, so the standard WebJobs Executing/Executed are unnecessary duplication. By omitting them, I'm seeing a 5% RPS gain in load tests, since we're effectively reducing ETW events by 50%.
I need these 2 bits of info to ensure the middleware logs have all the required details - they're missing name/id currently.
There was a problem hiding this comment.
This may be problematic. I'm pretty sure we have detectors that rely on those Executing/Executed events and we've added some sample queries in Log Analytics that rely on them as well. We'd need a good way to see all your executions (HTTP or other) in a query and try to keep it as simple as possible.
There was a problem hiding this comment.
I've removed this change from the PR, until we decide on it.
| @@ -145,23 +151,36 @@ public async Task<ITriggerData> BindAsync(object value, ValueBindingContext cont | |||
| valueProvider = new HttpRequestValueBinder(_parameter, request, invokeString); | |||
| } | |||
|
|
|||
| var bindingData = await GetBindingDataAsync(request, valueProvider); | |||
| object poco = null; | |||
There was a problem hiding this comment.
To make the binding data lazy, I have to hoist the async evaluation portions out, since the TriggerData.BindingData interface isn't async.
| // apply some invocation details to the request that can be used later | ||
| // in the http request pipeline | ||
| request.HttpContext.Items[ScopeKeys.FunctionInvocationId] = context.FunctionInstanceId.ToString(); | ||
| request.HttpContext.Items[ScopeKeys.FunctionName] = context.FunctionContext.MethodName; |
There was a problem hiding this comment.
This may be problematic. I'm pretty sure we have detectors that rely on those Executing/Executed events and we've added some sample queries in Log Analytics that rely on them as well. We'd need a good way to see all your executions (HTTP or other) in a query and try to keep it as simple as possible.
Making http trigger BindingData initialization lazy, constructing the data dictionary only when it's asked for by the binding pipeline. Also adding a couple new request context items that I'll be leveraging in an imminent functions host PR.
For an HttpTrigger function with no input/output bindings (ExecutionContext binding notwithstanding - it doesn't use binding data), making this lazy saves about 5 dictionary creations, an object creation (SysBindingData), as well as all the initialization/precedence logic for initializing those dictionaries, for each request.