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

Add comments pointing to the source of logging dimension names #4613

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -20,9 +20,11 @@ public static string[] PrepareNormalizedHeaderNames(KeyValuePair<string, DataCla
}

[SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase",
Justification = "Normalization to lower case is required by OTel's semantic conventions")]
Justification = "Normalization to lower case is required by OTel semantic conventions")]
private static string Normalize(string header)
{
// Normalization of header names required by OTel semantic-conventions:
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes.
return header.ToLowerInvariant();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ namespace Microsoft.AspNetCore.Diagnostics.Logging;
/// <summary>
/// Constants used for incoming HTTP request logging tags.
/// </summary>
// There is no OTel semantic convention for HTTP logs, therefore we are using
// semantic conventions for HTTP spans:
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md.
// Some dimensions (e.g. Host and Method) were not renamed, though they had corresponding
// names in the semantic conventions. The reason is that they are coming from ASP.NET Core 8
// HttpLogging component, therefore we cannot rename them.
Comment on lines +12 to +17
Copy link
Member

Choose a reason for hiding this comment

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

Is this something we'd want end-users to see in the docs (e.g., at learn.microsoft), or is this for the maintainers only?
Ditto for the other comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't mean to include this comment as public documentation, and my initial intent was to include it as a reference and justification "WHY" only for maintainers. It is part of public API though, and such a comment/remark could be useful for users.

Copy link
Contributor Author

@iliar-turdushev iliar-turdushev Oct 24, 2023

Choose a reason for hiding this comment

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

Regarding web doc, e.g. learn.microsoft, I think that we'll include into the doc a list of log dimensions and remark that some of those dimensions are following OTel semantic-conventions. As I understand, web doc will be developed separately, right?

public static class HttpLoggingTagNames
{
/// <summary>
Expand All @@ -19,6 +25,7 @@ public static class HttpLoggingTagNames
/// <summary>
/// HTTP Host.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-server-semantic-conventions.
public const string Host = "server.address";

/// <summary>
Expand All @@ -34,11 +41,13 @@ public static class HttpLoggingTagNames
/// <summary>
/// HTTP Request Headers prefix.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes.
public const string RequestHeaderPrefix = "http.request.header.";

/// <summary>
/// HTTP Response Headers prefix.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes.
public const string ResponseHeaderPrefix = "http.response.header.";

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ public static class ApplicationEnricherTags
/// <summary>
/// Application name.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/README.md#service.
public const string ApplicationName = "service.name";

/// <summary>
/// Environment name.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/deployment-environment.md#deployment.
public const string EnvironmentName = "deployment.environment";

/// <summary>
Expand All @@ -29,6 +31,7 @@ public static class ApplicationEnricherTags
/// <summary>
/// Build version.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/README.md#service.
public const string BuildVersion = "service.version";

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ public static class ProcessEnricherTagNames
/// <summary>
/// Process ID.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/process.md#process.
public const string ProcessId = "process.pid";

/// <summary>
/// Thread ID.
/// </summary>
// See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attributes.md#general-thread-attributes.
public const string ThreadId = "thread.id";

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ namespace Microsoft.Extensions.Logging;

internal sealed partial class ExtendedLogger : ILogger
{
// We are using OTel semantic conventions for Exceptions in Logs:
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/exceptions/exceptions-logs.md.
private const string ExceptionType = "exception.type";
private const string ExceptionMessage = "exception.message";
private const string ExceptionStackTrace = "exception.stacktrace";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ namespace Microsoft.Extensions.Http.Logging;
/// <summary>
/// Constants used for HTTP client logging tags.
/// </summary>
// There is no OTel semantic convention for HTTP logs, therefore we are using semantic conventions for HTTP spans:
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md.
// "Duration", "RequestBody" and "ResponseBody" are not part of the semantic conventions though.
public static class HttpClientLoggingTagNames
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ public static void AddResponseHeaders(this LoggerMessageState state, List<KeyVal
}

[SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase",
Justification = "Normalization to lower case is required by OTel's semantic conventions")]
Justification = "Normalization to lower case is required by OTel semantic conventions")]
private static string Normalize(string header)
{
// Normalization of header names required by OTel semantic-conventions:
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes.
return header.ToLowerInvariant();
}
}