Removing ClearProvidersExceptFunctionProviders causes the Application Insights OperationName to change

[pim-simons]

Describe the bug
Since the package Arcus.Observability.Telemetry.AzureFunctions has been marked as obsolete I have removed this package from my Azure Function. This meant I had to remove the ClearProvidersExceptFunctionProviders functionality from the startup.

As a result the OperationName in Application Insights has changed from for example GET /api/person/12345 to the FunctionName defined in [FunctionName("MyFunction")].
Even specifying the OperationName to a custom value in the LogRequest results in the FunctionName and not the custom value.
This means the overview of performance in Application Insights becomes unusable as it only shows the FunctionName instead of the API path.

To Reproduce
Remove ClearProvidersExceptFunctionProviders functionality from the startup.

Expected behavior
The OperationName stays the same.

Additional context
Latest versions of all the Arcus.Observability packages.

[stijnmoreels]

Caused in v2.4.

[stijnmoreels]

Ok, @pim-simons , I found the reason why this happened.

The ClearProvidersExceptFunctionsProviders made sure that the ApplicationInsightsLoggerProvider was removed from the registered services. On start-up, the Azure Functions application has 3 logger providers registered:

• FunctionsHostFileLoggerProvider
• HostFileLoggerProvider
• ApplicationInsightsLoggerProvider

This last one was removed by the ClearProvidersExceptionFunctionsProviders. What this provider did, was retrieving the send-out telemetry from a TelemetryClient, or in our case, via a Serilog configuration, and append additional information to this. So, in this case, the Function's name was added. You can see this here in the source where the operation name is been set to the Function's name.

So, in conclusion, the ClearProvidersExceptFunctionsProviders does provide a necessary functionality here, as the request tracking will be different without it.

@fgheysels , some insights from you would be helpful.

[pim-simons]

@stijnmoreels that is some great investigation work 👍🏻
So how do we go forward now, reinstate the Arcus.Observability.Telemetry.AzureFunctions package and not marking it as obsolete? Any other approaches?

[stijnmoreels]

I was wondering if we can make sure that we re-implement the function to only remove this Application Insights logger provider, instead of the current implementation (which loops and removes something other than default logger providers).

Maybe this could be in a different extension method for clearer understanding, but we do, indeed, would require to 'undeprecated' the current Azure Functions package.

[stijnmoreels]

If there is a way to disable this request tracking altering without fully removing the built-in Application Insights tracking (like dependencies and metrics), ... maybe that would be cool, but haven't found it yet.

[stijnmoreels]

Adding @fgheysels to this for his insights.

[fgheysels]

I find it strange that, by specifying the ClearProviders method, you actually had request traces of your Function in App Insights. We have removed this functionality, because by adding ClearProviders, the Function invocations where not visible in App Insights. (Certainly for timer triggered functions f.i).

See also #336

I think the best thing to do, is to provide an extension method with a name which clearly reveals its intention, which would only be used in http triggered Azure Functions then I guess. For instance:

.WithApiRequestPathLogging()

or something. I think it is important that the name guides users to only use this in a specific case, and not by default for all Azure Functions.

I'm also wondering what other side-effects removing the AppInsights provider might cause.

[stijnmoreels]

I find it strange that, by specifying the ClearProviders method, you actually had request traces of your Function in App Insights. We have removed this functionality, because by adding ClearProviders, the Function invocations where not visible in App Insights. (Certainly for timer triggered functions f.i).

The request telemetry was written by our own LogRequest logging, so it makes sense that the removing Microsoft's ApplicationInsightsLoggerProvider doesn't remove our call to Application Insights.

I think the best thing to do, is to provide an extension method with a name which clearly reveals its intention, which would only be used in http triggered Azure Functions then I guess.

We will use request tracking for non-HTTP functions as well, like Azure Service Bus triggered functions. We will use a different logging extension of course (LogServiceBusRequest) but behind the scenes, the same kind of type will be created, and so, the same kind of problem could arise. So, maybe this should not be limited to HTTP triggers.

[fgheysels]

We will use request tracking for non-HTTP functions as well, like Azure Service Bus triggered functions. We will use a different logging extension of course (LogServiceBusRequest) but behind the scenes, the same kind of type will be created, and so, the same kind of problem could arise. So, maybe this should not be limited to HTTP triggers.

As far as I know, when you configure Application Insights in an Azure Function, and your Function uses a ServiceBus trigger, this request-logging is already build in in App Insights. So I wonder why would we use Arcus for that ?

[stijnmoreels]

As far as I know, when you configure Application Insights in an Azure Function, and your Function uses a ServiceBus trigger, this request-logging is already build in in App Insights. So I wonder why would we use Arcus for that ?

Because the Arcus variant makes sure that we can log the dependency ID from the previous call in our request telemetry tracking, and so, make sure that the service-to-service correlation is streamlined.

[stijnmoreels]

We could add a new extension (in the like of logging.RemoveMicrosoftApplicationInsightsLogger) that removes the ApplicationInsightsLoggerProvider from the registered services. That way, it's more clear what's happening which was a problem in ClearProviders... in the previous one, and we're still sure that the operation name is fixed (and other trace problems like #359 ). Thoughts @pim-simons , @fgheysels ?

[pim-simons]

We could add a new extension (in the like of logging.RemoveMicrosoftApplicationInsightsLogger) that removes the ApplicationInsightsLoggerProvider from the registered services. That way, it's more clear what's happening which was a problem in ClearProviders... in the previous one, and we're still sure that the operation name is fixed (and other trace problems like #359 ). Thoughts @pim-simons , @fgheysels ?

So if I understand correctly, it will only be a change in the name of the extension to make it more clear what it does? The actual functionality will stay the same?
For me that sounds like a good solution

[stijnmoreels]

So if I understand correctly, it will only be a change in the name of the extension to make it more clear what it does? The actual functionality will stay the same? For me that sounds like a good solution

Yeah, that's right. The implementation will only be more robust and it will be more clear what it does by the name of the extension. 👍

[fgheysels]

I can live with the suggested name, it reveals what it is doing. However, it doesn't communicate on why you want to do it, but that can be handled by documentation.
I think it would be nice if we could come up with a name that also reveals something on the why, but given the broad spectrum on where you could use it, I cannot come up with a better name, so let's stick with the suggested name and elaborate on why you want to call that method in the documentation

[stijnmoreels]

We could also create our own UseSerilog variant that does this for us, but that could also maybe hide too much in the background.

We could also add a warning message when the ApplicationInsightsLoggerProvider is still in the registered providers so to inform users. Maybe that's also a good indication besides the docs of course.