Skip to content
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

use named templating in log messages #1797

Merged
merged 3 commits into from
Jul 31, 2023
Merged

Conversation

julienforet
Copy link
Contributor

When using log destructuration having logs with {0}, {1} in templates can be an issue as these have different types for each log messages

@julienforet
Copy link
Contributor Author

@microsoft-github-policy-service agree

Copy link
Member

@vicancy vicancy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @julienforet, thanks for the contribution! The PR looks great to me. Looks like there is a test failure, could you help address that?

[xUnit.net 00:00:09.11]     Microsoft.Azure.SignalR.Tests.MessageLogTests.MessageLogTest [FAIL]
  Failed Microsoft.Azure.SignalR.Tests.MessageLogTests.MessageLogTest [12 ms]
  Error Message:
   System.FormatException : Input string was not in a correct format.
  Stack Trace:
     at System.Text.ValueStringBuilder.ThrowFormatInvalidString()
   at System.Text.ValueStringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ReadOnlySpan`1 args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ReadOnlySpan`1 args)
   at System.String.Format(String format, Object arg0)
   at Microsoft.Azure.SignalR.Tests.MessageLogTests.MessageLogTest() in /Users/runner/work/azure-signalr/azure-signalr/test/Microsoft.Azure.SignalR.Tests/Logging/MessageLogTests.cs:line 22
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

@julienforet
Copy link
Contributor Author

Sorry for the delay, I fixed it

@vicancy vicancy merged commit 4d1a1e5 into Azure:dev Jul 31, 2023
7 of 9 checks passed
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