Hosting logging needs to use structured logging instead of format strings

[Eilon]

E.g. here:

Hosting/src/Microsoft.AspNetCore.Hosting/Internal/HostingLoggerExtensions.cs

Line 269 in 3ea44c6

_cachedToString = $"Request finished in {_elapsed.TotalMilliseconds}ms {_httpContext.Response.StatusCode} {_httpContext.Response.ContentType}";

It's using a formatted string, which means that structured logging doesn't get to see what's in there. It also means that the formatting is being done in the current locale of the thread, which is unlikely to achieve the desired effect.

The logging calls need to instead use structured logging.

[Tratcher]

The structured part is here:

Hosting/src/Microsoft.AspNetCore.Hosting/Internal/HostingLoggerExtensions.cs

Lines 241 to 257 in 3ea44c6

	public KeyValuePair<string, object> this[int index]
	{
	get
	{
	switch (index)
	{
	case 0:
	return new KeyValuePair<string, object>("ElapsedMilliseconds", _elapsed.TotalMilliseconds);
	case 1:
	return new KeyValuePair<string, object>("StatusCode", _httpContext.Response.StatusCode);
	case 2:
	return new KeyValuePair<string, object>("ContentType", _httpContext.Response.ContentType);
	default:
	throw new IndexOutOfRangeException(nameof(index));
	}
	}
	}

ToString is just the fallback.

[Eilon]

Hmm according to the linked bug aspnet/Mvc#3986 the data is formatted via the ToString in the log entry - that still looks wrong. Good to know that the structured data is available, though.

[Tratcher]

Some of the loggers don't consume the structured data, they just call ToString. @BrennanConroy?

[BrennanConroy]

Its up to the logger to decide if it wants to use structured data and currently none of our loggers use structured logging anymore.

[Eilon]

Perhaps not, but the formatting to a string needs to be done with InvariantCulture in anything that we ship. Is that in Logging maybe?

[Tratcher]

The formatting is happening inline in ToString.

[Eilon]

Ah then I think that should change, no? Just need to pass in the right culture (InvariantCulture).

[Tratcher]

Well, it means you can't use the $"{myVar}" syntax anymore.

[Eilon]

Hardly the end of the world 😄