Skip to content

Conversation

@CodeBlanch
Copy link
Contributor

@CodeBlanch CodeBlanch commented Jul 26, 2023

This PR updates the LogMetric, LogFunctionResult, LogFunctionResultAggregate ILogger extensions to use TStates which support structured logging:

  • {OriginalFormat} attribute is added as the last KVP which contains a template for these logs.
  • A formatter is provided which will generate formatted messages based on the template if called.

The reason for this is OpenTelemetry is implemented as an ILoggerProvider and it kind of barfs on the existing states which do not implement the typical structured logging state contract. Implementing this structure will make these logs work better with OpenTelemetry and likely many other ILoggerProvider implementations. See: Azure/azure-sdk-for-net#37832

This is similar to: dotnet/aspnetcore#45253

@RohitRanjanMS
Copy link
Member

This looks good.
image

I will check at other data sources and confirm.

We ignore these `Aggregate`, `Result` and `Metric` logs and don't log it anywhere. We rely the on formatted message to ignore these logs (one example here https://github.com/Azure/azure-functions-host/blob/b42df4d4e63ab87f7a737f5a65dadf3eb7ba7de4/src/WebJobs.Script/Diagnostics/FileLogger.cs#L50). 

Setting the formatter to `null` so that we still ignore these.
Updated formatting.
@RohitRanjanMS
Copy link
Member

Hi @CodeBlanch , I have made some changes based on our last conversation. Also updated formatting to align with rest of the logs. If you are ok with these changes, I will merge the code and create a release.

@CodeBlanch
Copy link
Contributor Author

@RohitRanjanMS Changes LGTM!

@RohitRanjanMS RohitRanjanMS merged commit 9ed638f into Azure:dev Aug 21, 2023
@cataggar
Copy link
Member

This got merged on August 21st, so it looks like it should be in 3.0.38 or higher, but none are yet published to https://www.nuget.org/packages/Microsoft.Azure.WebJobs/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants