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 all 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using Microsoft.Extensions.Compliance.Classification;

internal static class HeaderNormalizer
{
public static string[] PrepareNormalizedHeaderNames(KeyValuePair<string, DataClassification>[] headers, string prefix)
{
var normalizedHeaders = new string[headers.Length];

for (int i = 0; i < headers.Length; i++)
{
normalizedHeaders[i] = prefix + Normalize(headers[i].Key);
}

return normalizedHeaders;
}

[SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase",
Justification = "Normalization to lower case is required by OTel's semantic conventions")]
private static string Normalize(string header)
{
return header.ToLowerInvariant();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,32 @@ internal sealed class HeaderReader
{
private readonly IRedactorProvider _redactorProvider;
private readonly KeyValuePair<string, DataClassification>[] _headers;
private readonly string[] _normalizedHeaders;

public HeaderReader(IDictionary<string, DataClassification> headersToLog, IRedactorProvider redactorProvider)
public HeaderReader(IDictionary<string, DataClassification> headersToLog, IRedactorProvider redactorProvider, string prefix)
{
_redactorProvider = redactorProvider;

_headers = headersToLog.Count == 0 ? [] : headersToLog.ToArray();
_normalizedHeaders = HeaderNormalizer.PrepareNormalizedHeaderNames(_headers, prefix);
}

public void Read(IHeaderDictionary headers, IList<KeyValuePair<string, object?>> logContext, string prefix)
public void Read(IHeaderDictionary headers, IList<KeyValuePair<string, object?>> logContext)
{
if (headers.Count == 0)
{
return;
}

foreach (var header in _headers)
for (int i = 0; i < _headers.Length; i++)
{
var header = _headers[i];

if (headers.TryGetValue(header.Key, out var headerValue))
{
var provider = _redactorProvider.GetRedactor(header.Value);
var redacted = provider.Redact(headerValue.ToString());
logContext.Add(new(prefix + header.Key, redacted));
logContext.Add(new(_normalizedHeaders[i], redacted));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ public HttpLoggingRedactionInterceptor(
_requestPathLogMode = EnsureRequestPathLoggingModeIsValid(optionsValue.RequestPathLoggingMode);
_parameterRedactionMode = optionsValue.RequestPathParameterRedactionMode;

_requestHeadersReader = new(optionsValue.RequestHeadersDataClasses, redactorProvider);
_responseHeadersReader = new(optionsValue.ResponseHeadersDataClasses, redactorProvider);
_requestHeadersReader = new(optionsValue.RequestHeadersDataClasses, redactorProvider, HttpLoggingTagNames.RequestHeaderPrefix);
_responseHeadersReader = new(optionsValue.ResponseHeadersDataClasses, redactorProvider, HttpLoggingTagNames.ResponseHeaderPrefix);

_excludePathStartsWith = optionsValue.ExcludePathStartsWith.ToArray();
}
Expand Down Expand Up @@ -126,7 +126,7 @@ public ValueTask OnRequestAsync(HttpLoggingInterceptorContext logContext)

if (logContext.TryDisable(HttpLoggingFields.RequestHeaders))
{
_requestHeadersReader.Read(context.Request.Headers, logContext.Parameters, HttpLoggingTagNames.RequestHeaderPrefix);
_requestHeadersReader.Read(context.Request.Headers, logContext.Parameters);
}

return default;
Expand All @@ -138,7 +138,7 @@ public ValueTask OnResponseAsync(HttpLoggingInterceptorContext logContext)

if (logContext.TryDisable(HttpLoggingFields.ResponseHeaders))
{
_responseHeadersReader.Read(context.Response.Headers, logContext.Parameters, HttpLoggingTagNames.ResponseHeaderPrefix);
_responseHeadersReader.Read(context.Response.Headers, logContext.Parameters);
}

// Don't enrich if we're not going to log any part of the response
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 @@ -2,7 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Frozen;
using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Compliance.Classification;
using Microsoft.Extensions.Compliance.Redaction;
Expand All @@ -17,7 +18,8 @@ namespace Microsoft.AspNetCore.Diagnostics.Logging;
/// </summary>
internal sealed class RequestHeadersLogEnricher : ILogEnricher
{
private readonly FrozenDictionary<string, DataClassification> _headersDataClasses;
private readonly KeyValuePair<string, DataClassification>[] _headersDataClasses;
private readonly string[] _normalizedHeaders;
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly IRedactorProvider? _redactorProvider;

Expand All @@ -33,11 +35,10 @@ public RequestHeadersLogEnricher(IHttpContextAccessor httpContextAccessor, IOpti
var opt = Throw.IfMemberNull(options, options.Value);
_httpContextAccessor = httpContextAccessor;

_headersDataClasses = opt.HeadersDataClasses.Count == 0
? FrozenDictionary<string, DataClassification>.Empty
: opt.HeadersDataClasses.ToFrozenDictionary(StringComparer.Ordinal);
_headersDataClasses = opt.HeadersDataClasses.Count == 0 ? [] : opt.HeadersDataClasses.ToArray();
_normalizedHeaders = HeaderNormalizer.PrepareNormalizedHeaderNames(_headersDataClasses, HttpLoggingTagNames.RequestHeaderPrefix);

if (_headersDataClasses.Count > 0)
if (_headersDataClasses.Length > 0)
{
_redactorProvider = Throw.IfNull(redactorProvider);
}
Expand All @@ -54,20 +55,22 @@ public void Enrich(IEnrichmentTagCollector collector)

var request = _httpContextAccessor.HttpContext.Request;

if (_headersDataClasses.Count == 0)
if (_headersDataClasses.Length == 0)
{
return;
}

if (_headersDataClasses.Count != 0)
if (_headersDataClasses.Length != 0)
{
foreach (var header in _headersDataClasses)
for (int i = 0; i < _headersDataClasses.Length; i++)
{
var header = _headersDataClasses[i];

if (request.Headers.TryGetValue(header.Key, out var headerValue) && !string.IsNullOrEmpty(headerValue))
{
var redactor = _redactorProvider!.GetRedactor(header.Value);
var redacted = redactor.Redact(headerValue);
collector.Add(header.Key, redacted);
collector.Add(_normalizedHeaders[i], redacted);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
{
"Member": "const string Microsoft.AspNetCore.Diagnostics.Logging.HttpLoggingTagNames.Host",
"Stage": "Stable",
"Value": "Host"
"Value": "server.address"
},
{
"Member": "const string Microsoft.AspNetCore.Diagnostics.Logging.HttpLoggingTagNames.Method",
Expand All @@ -51,7 +51,7 @@
{
"Member": "const string Microsoft.AspNetCore.Diagnostics.Logging.HttpLoggingTagNames.RequestHeaderPrefix",
"Stage": "Stable",
"Value": "RequestHeader."
"Value": "http.request.header."
},
{
"Member": "const string Microsoft.AspNetCore.Diagnostics.Logging.HttpLoggingTagNames.ResponseBody",
Expand All @@ -61,7 +61,7 @@
{
"Member": "const string Microsoft.AspNetCore.Diagnostics.Logging.HttpLoggingTagNames.ResponseHeaderPrefix",
"Stage": "Stable",
"Value": "ResponseHeader."
"Value": "http.response.header."
},
{
"Member": "const string Microsoft.AspNetCore.Diagnostics.Logging.HttpLoggingTagNames.StatusCode",
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 @@ -26,22 +26,22 @@
{
"Member": "const string Microsoft.Extensions.Diagnostics.Enrichment.ApplicationEnricherTags.ApplicationName",
"Stage": "Stable",
"Value": "AppName"
"Value": "service.name"
},
{
"Member": "const string Microsoft.Extensions.Diagnostics.Enrichment.ApplicationEnricherTags.BuildVersion",
"Stage": "Stable",
"Value": "CloudRoleVer"
"Value": "service.version"
},
{
"Member": "const string Microsoft.Extensions.Diagnostics.Enrichment.ApplicationEnricherTags.DeploymentRing",
"Stage": "Stable",
"Value": "CloudDeploymentRing"
"Value": "DeploymentRing"
},
{
"Member": "const string Microsoft.Extensions.Diagnostics.Enrichment.ApplicationEnricherTags.EnvironmentName",
"Stage": "Stable",
"Value": "CloudEnv"
"Value": "deployment.environment"
}
],
"Properties": [
Expand Down Expand Up @@ -260,12 +260,12 @@
{
"Member": "const string Microsoft.Extensions.Diagnostics.Enrichment.ProcessEnricherTagNames.ProcessId",
"Stage": "Stable",
"Value": "pid"
"Value": "process.pid"
},
{
"Member": "const string Microsoft.Extensions.Diagnostics.Enrichment.ProcessEnricherTagNames.ThreadId",
"Stage": "Stable",
"Value": "tid"
"Value": "thread.id"
}
],
"Properties": [
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
Loading