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 the same naming rules for the names of logging dimensions #4545

Merged
merged 21 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e0cebd1
Fixed naming of HttpClient logs, Http Middleware logs, and LoggerMess…
iliar-turdushev Oct 9, 2023
5e63fca
Renamed enrichment properties
iliar-turdushev Oct 9, 2023
ec04952
ComplexObjectLogging: use "." instead of "_"
iliar-turdushev Oct 10, 2023
0fb8cd8
Fixed a test
iliar-turdushev Oct 10, 2023
6fcf38a
Merge with release/8.0
iliar-turdushev Oct 10, 2023
8f00e79
Make dimension names in HTTP Client Logging consistent with HTTP Logging
iliar-turdushev Oct 10, 2023
c93c1a0
Renamed AppEnricher's dimensions
iliar-turdushev Oct 10, 2023
58afe44
Addressing PR comments
iliar-turdushev Oct 11, 2023
8ea197b
Reverted changes to StackTrace logging
iliar-turdushev Oct 12, 2023
274efcd
Apply semantic-conventions to Application and Process enrichers
iliar-turdushev Oct 12, 2023
2f324a7
Apply semantic-conventions to HttpLogging
iliar-turdushev Oct 13, 2023
00c9cd3
Dimension names in HttpClient Logging
iliar-turdushev Oct 15, 2023
8282a78
Inlined PropertySeparator and NonNullablePropertySeparator constants
iliar-turdushev Oct 17, 2023
2992316
Adjusted code coverage
iliar-turdushev Oct 19, 2023
91968e3
Header normalization
iliar-turdushev Oct 19, 2023
466b9a1
HttpLogging header name normalization
iliar-turdushev Oct 20, 2023
fe0f620
Normalize headers added by RequestHeadersLogEnricher
iliar-turdushev Oct 20, 2023
dd45788
Header name normalization in HttpClientLogging
iliar-turdushev Oct 20, 2023
cd73076
Merge with main
iliar-turdushev Oct 20, 2023
7fc9a02
Updated API json files
iliar-turdushev Oct 20, 2023
f44ed27
Header name normalization now doesn't require replacing '-' with '_'
iliar-turdushev Oct 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc
{
if (!member.HasDataClassification)
{
var propName = PropertyChainToString(propertyChain, member, "_", omitReferenceName: p.OmitReferenceName);
var propName = PropertyChainToString(propertyChain, member, ".", omitReferenceName: p.OmitReferenceName);
var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: ".");

var ts = ShouldStringifyProperty(member)
Expand Down Expand Up @@ -367,7 +367,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc
{
if (member.HasDataClassification)
{
var propName = PropertyChainToString(propertyChain, member, "_", omitReferenceName: p.OmitReferenceName);
var propName = PropertyChainToString(propertyChain, member, ".", omitReferenceName: p.OmitReferenceName);
var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: ".");

var value = ShouldStringifyProperty(member)
Expand All @@ -391,7 +391,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc
{
if (member.HasDataClassification)
{
var propName = PropertyChainToString(propertyChain, member, "_", omitReferenceName: p.OmitReferenceName);
var propName = PropertyChainToString(propertyChain, member, ".", omitReferenceName: p.OmitReferenceName);
var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: ".");

var value = ShouldStringifyProperty(member)
Expand All @@ -418,7 +418,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc
}
else
{
var propName = PropertyChainToString(propertyChain, member, "_", omitReferenceName: p.OmitReferenceName);
var propName = PropertyChainToString(propertyChain, member, ".", omitReferenceName: p.OmitReferenceName);
var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: ".");

var ts = ShouldStringifyProperty(member)
Expand Down
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static class HttpLoggingTagNames
/// <summary>
/// HTTP Host.
/// </summary>
public const string Host = "Host";
public const string Host = "server.address";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// HTTP Method.
Expand All @@ -34,12 +34,12 @@ public static class HttpLoggingTagNames
/// <summary>
/// HTTP Request Headers prefix.
/// </summary>
public const string RequestHeaderPrefix = "RequestHeader.";
public const string RequestHeaderPrefix = "http.request.header.";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// HTTP Response Headers prefix.
/// </summary>
public const string ResponseHeaderPrefix = "ResponseHeader.";
public const string ResponseHeaderPrefix = "http.response.header.";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// HTTP Request Body.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,12 +198,12 @@ private void CopyTo(Box<T> to)
}
}

[LoggerMessage(LogLevel.Debug, "Can't parse header '{headerName}' due to '{error}'.")]
[LoggerMessage(LogLevel.Debug, "Can't parse header '{HeaderName}' due to '{Error}'.")]
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved
private partial void LogParsingError(string headerName, string error);

[LoggerMessage(LogLevel.Debug, "Using a default value for header '{headerName}'.")]
[LoggerMessage(LogLevel.Debug, "Using a default value for header '{HeaderName}'.")]
private partial void LogDefaultUsage(string headerName);

[LoggerMessage(LogLevel.Debug, "Header '{headerName}' not found.")]
[LoggerMessage(LogLevel.Debug, "Header '{HeaderName}' not found.")]
private partial void LogNotFound(string headerName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@ public static class ApplicationEnricherTags
/// <summary>
/// Application name.
/// </summary>
public const string ApplicationName = "AppName";
public const string ApplicationName = "service.name";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Environment name.
/// </summary>
public const string EnvironmentName = "CloudEnv";
public const string EnvironmentName = "deployment.environment";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Deployment ring.
/// </summary>
public const string DeploymentRing = "CloudDeploymentRing";
public const string DeploymentRing = "DeploymentRing";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Build version.
/// </summary>
public const string BuildVersion = "CloudRoleVer";
public const string BuildVersion = "service.version";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets a list of all dimension names.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ public static class ProcessEnricherTagNames
/// <summary>
/// Process ID.
/// </summary>
public const string ProcessId = "pid";
public const string ProcessId = "process.pid";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Thread ID.
/// </summary>
public const string ThreadId = "tid";
public const string ThreadId = "thread.id";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets a list of all dimension names.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Microsoft.Extensions.Logging;

public partial class LoggerMessageState : ITagCollector
{
private const char Separator = '_';
private const char Separator = '.';
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <inheritdoc />
void ITagCollector.Add(string tagName, object? tagValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ namespace Microsoft.Extensions.Diagnostics.HealthChecks;

internal static partial class Log
{
[LoggerMessage(0, LogLevel.Warning, "Process reporting unhealthy: {status}. Health check entries are {entries}")]
[LoggerMessage(0, LogLevel.Warning, "Process reporting unhealthy: {Status}. Health check entries are {Entries}")]
public static partial void Unhealthy(
ILogger logger,
HealthStatus status,
StringBuilder entries);

[LoggerMessage(1, LogLevel.Debug, "Process reporting healthy: {status}.")]
[LoggerMessage(1, LogLevel.Debug, "Process reporting healthy: {Status}.")]
public static partial void Healthy(
ILogger logger,
HealthStatus status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ internal static partial class Log
[LoggerMessage(1, LogLevel.Error, "Unable to gather utilization statistics.")]
public static partial void HandledGatherStatisticsException(ILogger logger, Exception e);

[LoggerMessage(2, LogLevel.Error, "Publisher `{publisher}` was unable to publish utilization statistics.")]
[LoggerMessage(2, LogLevel.Error, "Publisher `{Publisher}` was unable to publish utilization statistics.")]
public static partial void HandlePublishUtilizationException(ILogger logger, Exception e, string publisher);
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
_host.Dispose();
}

[LoggerMessage(0, LogLevel.Warning, "FakeHostOptions.TimeToLive set to {timeToLive} is up, disposing the host.")]
[LoggerMessage(0, LogLevel.Warning, "FakeHostOptions.TimeToLive set to {TimeToLive} is up, disposing the host.")]
private partial void LogTimeToLiveUp(TimeSpan timeToLive);

[LoggerMessage(1, LogLevel.Information, "Debugger is attached. The host won't be automatically disposed.")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void Enrich(IEnrichmentTagCollector collector, HttpRequestMessage request
AppendCheckpoints(lc, stringBuilder);
}

collector.Add("latencyInfo", stringBuilder.ToString());
collector.Add("LatencyInfo", stringBuilder.ToString());

_builderPool.Return(stringBuilder);
}
Expand Down
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -14,47 +14,47 @@ public static class HttpClientLoggingTagNames
/// <summary>
/// HTTP Request duration.
/// </summary>
public const string Duration = "duration";
public const string Duration = "Duration";

/// <summary>
/// HTTP Host.
/// </summary>
public const string Host = "httpHost";
public const string Host = "server.address";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// HTTP Method.
/// </summary>
public const string Method = "httpMethod";
public const string Method = "http.request.method";

/// <summary>
/// HTTP Path.
/// </summary>
public const string Path = "httpPath";
public const string Path = "url.path";

/// <summary>
/// HTTP Request Body.
/// </summary>
public const string RequestBody = "httpRequestBody";
public const string RequestBody = "RequestBody";

/// <summary>
/// HTTP Response Body.
/// </summary>
public const string ResponseBody = "httpResponseBody";
public const string ResponseBody = "ResponseBody";
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// HTTP Request Headers prefix.
/// </summary>
public const string RequestHeaderPrefix = "httpRequestHeader_";
public const string RequestHeaderPrefix = "http.request.header.";

/// <summary>
/// HTTP Response Headers prefix.
/// </summary>
public const string ResponseHeaderPrefix = "httpResponseHeader_";
public const string ResponseHeaderPrefix = "http.response.header.";

/// <summary>
/// HTTP Status Code.
/// </summary>
public const string StatusCode = "httpStatusCode";
public const string StatusCode = "http.response.status_code";

/// <summary>
/// Gets a list of all tag names.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ namespace Microsoft.Extensions.Http.Logging.Internal;
/// Logs <see cref="HttpRequestMessage"/>, <see cref="HttpResponseMessage"/> and the exceptions due to errors of request/response.
/// </summary>
[SuppressMessage("Major Code Smell", "S109:Magic numbers should not be used", Justification = "Event ID's.")]
internal static partial class Log
internal static class Log
{
internal const string OriginalFormat = "{OriginalFormat}";
internal const string OriginalFormatValue = "{httpMethod} {httpHost}/{httpPath}";
private const string NullString = "(null)";

private const int MinimalPropertyCount = 4;

Expand All @@ -28,10 +28,10 @@ internal static partial class Log
$"{{{HttpClientLoggingTagNames.Method}}} {{{HttpClientLoggingTagNames.Host}}}/{{{HttpClientLoggingTagNames.Path}}}";

private const string LoggerContextMissingMessage =
$"The logger couldn't read its context for {{requestState}} request: {{{HttpClientLoggingTagNames.Method}}} {{{HttpClientLoggingTagNames.Host}}}";
$"The logger couldn't read its context for {{RequestState}} request: {{{HttpClientLoggingTagNames.Method}}} {{{HttpClientLoggingTagNames.Host}}}";

private const string EnrichmentErrorMessage =
"An error occurred in enricher '{enricherType}' while enriching the logger context for request: " +
"An error occurred in enricher '{Enricher}' while enriching the logger context for request: " +
$"{{{HttpClientLoggingTagNames.Method}}} {{{HttpClientLoggingTagNames.Host}}}/{{{HttpClientLoggingTagNames.Path}}}";

private static readonly Func<LoggerMessageState, Exception?, string> _originalFormatValueFMTFunc = OriginalFormatValueFMT;
Expand All @@ -46,17 +46,114 @@ public static void OutgoingRequestError(ILogger logger, LogRecord record, Except
OutgoingRequest(logger, LogLevel.Error, 2, nameof(OutgoingRequestError), record, exception);
}

[LoggerMessage(LogLevel.Error, RequestReadErrorMessage)]
public static partial void RequestReadError(ILogger logger, Exception exception, HttpMethod httpMethod, string? httpHost, string? httpPath);
public static void RequestReadError(ILogger logger, Exception exception, HttpMethod method, string? host, string? path)
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved
{
var state = LoggerMessageHelper.ThreadLocalState;

_ = state.ReserveTagSpace(4);
state.TagArray[3] = new(HttpClientLoggingTagNames.Method, method);
state.TagArray[2] = new(HttpClientLoggingTagNames.Host, host);
state.TagArray[1] = new(HttpClientLoggingTagNames.Path, path);
state.TagArray[0] = new(OriginalFormat, RequestReadErrorMessage);
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

logger.Log(
LogLevel.Error,
new(0, nameof(RequestReadError)),
state,
exception,
static (s, _) =>
{
var method = s.TagArray[3].Value ?? NullString;
var host = s.TagArray[2].Value ?? NullString;
var path = s.TagArray[1].Value ?? NullString;
return FormattableString.Invariant(
$"An error occurred while reading the request data to fill the logger context for request: {method} {host}/{path}");
});

state.Clear();
}

[LoggerMessage(LogLevel.Error, ResponseReadErrorMessage)]
public static partial void ResponseReadError(ILogger logger, Exception exception, HttpMethod httpMethod, string httpHost, string httpPath);
public static void ResponseReadError(ILogger logger, Exception exception, HttpMethod method, string host, string path)
{
var state = LoggerMessageHelper.ThreadLocalState;

[LoggerMessage(LogLevel.Error, LoggerContextMissingMessage)]
public static partial void LoggerContextMissing(ILogger logger, Exception? exception, string requestState, HttpMethod httpMethod, string? httpHost);
_ = state.ReserveTagSpace(4);
state.TagArray[3] = new(HttpClientLoggingTagNames.Method, method);
state.TagArray[2] = new(HttpClientLoggingTagNames.Host, host);
state.TagArray[1] = new(HttpClientLoggingTagNames.Path, path);
state.TagArray[0] = new(OriginalFormat, ResponseReadErrorMessage);
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

logger.Log(
LogLevel.Error,
new(0, nameof(ResponseReadError)),
state,
exception,
static (s, _) =>
{
var method = s.TagArray[3].Value ?? NullString;
var host = s.TagArray[2].Value ?? NullString;
var path = s.TagArray[1].Value ?? NullString;
return FormattableString.Invariant(
$"An error occurred while reading the response data to fill the logger context for request: {method} {host}/{path}");
});

state.Clear();
}

public static void LoggerContextMissing(ILogger logger, Exception? exception, string requestState, HttpMethod method, string? host)
{
var state = LoggerMessageHelper.ThreadLocalState;

[LoggerMessage(LogLevel.Error, EnrichmentErrorMessage)]
public static partial void EnrichmentError(ILogger logger, Exception exception, string? enricherType, HttpMethod httpMethod, string httpHost, string httpPath);
_ = state.ReserveTagSpace(4);
state.TagArray[3] = new("RequestState", requestState);
state.TagArray[2] = new(HttpClientLoggingTagNames.Method, method);
state.TagArray[1] = new(HttpClientLoggingTagNames.Host, host);
state.TagArray[0] = new(OriginalFormat, LoggerContextMissingMessage);

logger.Log(
LogLevel.Error,
new(0, nameof(LoggerContextMissing)),
state,
exception,
(s, _) =>
{
var requestState = s.TagArray[3].Value ?? NullString;
var method = s.TagArray[2].Value ?? NullString;
var host = s.TagArray[1].Value ?? NullString;
return FormattableString.Invariant($"The logger couldn't read its context for {requestState} request: {method} {host}");
});

state.Clear();
}

public static void EnrichmentError(ILogger logger, Exception exception, string? enricher, HttpMethod method, string host, string path)
{
var state = LoggerMessageHelper.ThreadLocalState;

_ = state.ReserveTagSpace(5);
state.TagArray[4] = new("Enricher", enricher);
state.TagArray[3] = new(HttpClientLoggingTagNames.Method, method);
state.TagArray[2] = new(HttpClientLoggingTagNames.Host, host);
state.TagArray[1] = new(HttpClientLoggingTagNames.Path, path);
state.TagArray[0] = new(OriginalFormat, EnrichmentErrorMessage);

logger.Log(
LogLevel.Error,
new(0, nameof(EnrichmentError)),
state,
exception,
(s, _) =>
{
var enricher = s.TagArray[4].Value ?? NullString;
var method = s.TagArray[3].Value ?? NullString;
var host = s.TagArray[2].Value ?? NullString;
var path = s.TagArray[1].Value ?? NullString;
return FormattableString.Invariant(
$"An error occurred in enricher '{enricher}' while enriching the logger context for request: {method} {host}/{path}");
});

state.Clear();
}

// Using the code below instead of generated logging method because we have a custom formatter and custom tag keys for headers.
private static void OutgoingRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ public void AtSymbolsTest()

collector.Clear();
AtSymbolsTestExtensions.M3(logger, LogLevel.Debug, o);
Assert.Equal("42", collector.LatestRecord.StructuredState!.GetValue("event_class"));
Assert.Equal("42", collector.LatestRecord.StructuredState!.GetValue("event.class"));

collector.Clear();
AtSymbolsTestExtensions.M5(logger, LogLevel.Debug, o);
Expand Down Expand Up @@ -892,7 +892,7 @@ public void FormattableTest()
collector.Clear();
FormattableTestExtensions.Method2(logger, new FormattableTestExtensions.ComplexObj());
Assert.Equal(1, collector.Count);
Assert.Equal("Formatted!", collector.LatestRecord.StructuredState!.GetValue("p1_P1"));
Assert.Equal("Formatted!", collector.LatestRecord.StructuredState!.GetValue("p1.P1"));

collector.Clear();
FormattableTestExtensions.Method3(logger, default);
Expand Down
Loading
Loading