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

HttpLogging redaction and enrichment #50163

Merged
merged 34 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
8addb4f
Proof of concept for extending http logging
Tratcher Jun 2, 2023
a40b2d8
Feedback, cleanup
Tratcher Jun 9, 2023
affbe55
Nits
Tratcher Jun 12, 2023
f051458
Add duration
Tratcher Jun 12, 2023
dedc429
Fix log, test
Tratcher Jun 13, 2023
429137a
Pool log contexts
Tratcher Jun 13, 2023
c01fa22
API review
Tratcher Aug 17, 2023
c99ea47
Cleanup, TimeProvider
Tratcher Aug 17, 2023
60b503f
Duration
Tratcher Aug 22, 2023
cc1fc7c
Write temp files to the current dir, it might be on a different/faste…
Tratcher Aug 22, 2023
613f38b
Cleanup
Tratcher Aug 22, 2023
2a67078
Tests
Tratcher Aug 23, 2023
9dec914
Fix CopyTo/Async #49989
Tratcher Aug 23, 2023
fb37ff9
Apply suggestions from code review
Tratcher Aug 24, 2023
ca1b2ec
Clarify code check error
Tratcher Aug 23, 2023
be501ae
Comments, copyto, TryOverride
Tratcher Aug 24, 2023
3e3a8ca
Usings
Tratcher Aug 24, 2023
6734921
Code review, comments, tests
Tratcher Aug 25, 2023
8f732ec
Restructuring
Tratcher Aug 25, 2023
98d8261
Note when the request body was not fully logged #49063
Tratcher Aug 25, 2023
607e628
Powershell fix?
Tratcher Aug 25, 2023
d319c7d
Revert "Clarify code check error"
Tratcher Aug 28, 2023
a846c79
Feedback
Tratcher Aug 28, 2023
e09a349
Feedback
Tratcher Aug 29, 2023
9c847fd
Combine logs, log even if an exception happens
Tratcher Aug 30, 2023
bf48e46
Clarify
Tratcher Aug 30, 2023
b73c3f7
Don't log empty response body
Tratcher Aug 30, 2023
0678c83
Getting cozy
Tratcher Aug 31, 2023
80b0a80
Simplify Duration
Tratcher Aug 31, 2023
40e35f0
Less pooling
Tratcher Aug 31, 2023
cee71fb
Clean up comments
Tratcher Aug 31, 2023
ec5aad6
Even less re-use
Tratcher Aug 31, 2023
c9b6c46
Clean up Unshipped.txt
Tratcher Aug 31, 2023
d12739d
Re-fix Unshipped
Tratcher Aug 31, 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
8 changes: 6 additions & 2 deletions eng/scripts/CodeCheck.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,10 @@ try {
}
}
# Check for changes in Unshipped in servicing branches
if ($targetBranch -like 'release*' -and $targetBranch -notlike '*preview*' -and $file -like '*PublicAPI.Unshipped.txt') {
if ($targetBranch -like 'release*'
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
-and $targetBranch -notlike '*preview*'
-and $targetBranch -notlike '*rc*'
-and $file -like '*PublicAPI.Unshipped.txt') {
$changedAPIBaselines.Add($file)
}
}
Expand All @@ -263,7 +266,8 @@ try {

if ($changedAPIBaselines.count -gt 0) {
LogError ("Detected modification to baseline API files. PublicAPI.Shipped.txt files should only " +
"be updated after a major release. See /docs/APIBaselines.md for more information.")
"be updated after a major release. PublicAPI.Unshipped.txt should only be updated in main or pre-release branches. "
+ "See /docs/APIBaselines.md for more information.")
LogError "Modified API baseline files:"
foreach ($file in $changedAPIBaselines) {
LogError $file
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.HttpLogging;

namespace HttpLogging.Sample;

internal class SampleHttpLoggingInterceptor : IHttpLoggingInterceptor
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
{
public ValueTask OnRequestAsync(HttpLoggingInterceptorContext logContext)
{
// Compare to ExcludePathStartsWith
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to reference ExcludePathStartsWith here? I don't think that folks reviewing this sample should take into account https://github.com/dotnet/extensions/blob/a387e38dc927035db5492b47e9607867da7d5005/src/Libraries/Microsoft.AspNetCore.Telemetry.Middleware/Logging/LoggingOptions.cs#L216

Copy link
Contributor

@xakep139 xakep139 Aug 25, 2023

Choose a reason for hiding this comment

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

I presume this sample targets external audience that might not be aware of dotnet/extensions

Copy link
Member Author

Choose a reason for hiding this comment

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

These samples are for our own usage. I have this one currently set up to mimic what we want to accomplish in extensions. I may clean out some of the references before merging.

if (!logContext.HttpContext.Request.Path.StartsWithSegments("/api"))
{
logContext.LoggingFields = HttpLoggingFields.None;
}

// Don't enrich if we're not going to log any part of the request
if (!logContext.IsAnyEnabled(HttpLoggingFields.Request))
{
return default;
}

if (logContext.TryOverride(HttpLoggingFields.RequestPath))
{
RedactPath(logContext);
}

if (logContext.TryOverride(HttpLoggingFields.RequestHeaders))
{
RedactRequestHeaders(logContext);
}

EnrichRequest(logContext);

return default;
}

private void RedactRequestHeaders(HttpLoggingInterceptorContext logContext)
{
foreach (var header in logContext.HttpContext.Request.Headers)
{
logContext.AddParameter(header.Key, "RedactedHeader"); // TODO: Redact header value
}
}

private void RedactResponseHeaders(HttpLoggingInterceptorContext logContext)
{
foreach (var header in logContext.HttpContext.Response.Headers)
{
logContext.AddParameter(header.Key, "RedactedHeader"); // TODO: Redact header value
}
}

public ValueTask OnResponseAsync(HttpLoggingInterceptorContext logContext)
{
// Don't enrich if we're not going to log any part of the response
if (!logContext.IsAnyEnabled(HttpLoggingFields.Response))
{
return default;
}

if (logContext.TryOverride(HttpLoggingFields.ResponseHeaders))
{
RedactResponseHeaders(logContext);
}

EnrichResponse(logContext);

return default;
}

private void EnrichResponse(HttpLoggingInterceptorContext logContext)
{
logContext.AddParameter("ResponseEnrichment", "Stuff");
}

private void EnrichRequest(HttpLoggingInterceptorContext logContext)
{
logContext.AddParameter("RequestEnrichment", "Stuff");
}

private void RedactPath(HttpLoggingInterceptorContext logContext)
{
logContext.AddParameter(nameof(logContext.HttpContext.Request.Path), "RedactedPath");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public void ConfigureServices(IServiceCollection services)
{
logging.LoggingFields = HttpLoggingFields.All;
});
services.AddHttpLoggingInterceptor<SampleHttpLoggingInterceptor>();
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
Expand Down
24 changes: 2 additions & 22 deletions src/Middleware/HttpLogging/src/BufferingStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public override int WriteTimeout
set => _innerStream.WriteTimeout = value;
}

public string GetString(Encoding? encoding)
public string GetString(Encoding encoding)
{
try
{
Expand All @@ -57,13 +57,6 @@ public string GetString(Encoding? encoding)
return "";
}

if (encoding == null)
{
// This method is used only for the response body
_logger.UnrecognizedMediaType("response");
return "";
}

// Only place where we are actually using the buffered data.
// update tail here.
_tail.End = _tailBytesBuffered;
Expand Down Expand Up @@ -295,15 +288,7 @@ public override void EndWrite(IAsyncResult asyncResult)
_innerStream.EndWrite(asyncResult);
}

public override void CopyTo(Stream destination, int bufferSize)
{
_innerStream.CopyTo(destination, bufferSize);
}

public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken)
{
return _innerStream.CopyToAsync(destination, bufferSize, cancellationToken);
}
// Do not override CopyTo/Async, they call Read/Async internally.

public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default)
{
Expand All @@ -314,9 +299,4 @@ public override ValueTask DisposeAsync()
{
return _innerStream.DisposeAsync();
}

public override int Read(Span<byte> buffer)
{
return _innerStream.Read(buffer);
}
}
3 changes: 3 additions & 0 deletions src/Middleware/HttpLogging/src/HttpLoggingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,7 @@ public static void ResponseLog(this ILogger logger, HttpResponseLog responseLog)

[LoggerMessage(7, LogLevel.Debug, "No Content-Type header for {Name} body.", EventName = "NoMediaType")]
public static partial void NoMediaType(this ILogger logger, string name);

[LoggerMessage(8, LogLevel.Information, "Response Duration: {Duration}ms, Body: {Body}", EventName = "ResponseDurationAndBody")]
public static partial void ResponseBodyWithDuration(this ILogger logger, string body, double duration);
}
11 changes: 8 additions & 3 deletions src/Middleware/HttpLogging/src/HttpLoggingFields.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ public enum HttpLoggingFields : long
/// </summary>
ResponseBody = 0x800,

/// <summary>
/// Log how long it took to process the request and return a response in total milliseconds.
/// </summary>
Duration = 0x1000,

/// <summary>
/// Flag for logging a collection of HTTP Request properties,
/// including <see cref="RequestPath"/>, <see cref="RequestProtocol"/>,
Expand All @@ -158,9 +163,9 @@ public enum HttpLoggingFields : long

/// <summary>
/// Flag for logging HTTP Response properties and headers.
/// Includes <see cref="ResponseStatusCode"/> and <see cref="ResponseHeaders"/>
/// Includes <see cref="ResponseStatusCode"/>, <see cref="ResponseHeaders"/>, and <see cref="Duration"/>.
/// </summary>
ResponsePropertiesAndHeaders = ResponseStatusCode | ResponseHeaders,
ResponsePropertiesAndHeaders = ResponseStatusCode | ResponseHeaders | Duration,

/// <summary>
/// Flag for logging the entire HTTP Request.
Expand All @@ -180,7 +185,7 @@ public enum HttpLoggingFields : long
/// Logging the response body has performance implications, as it requires buffering
/// the entire response body up to <see cref="HttpLoggingOptions.ResponseBodyLogLimit"/>.
/// </summary>
Response = ResponseStatusCode | ResponseHeaders | ResponseBody,
Response = ResponsePropertiesAndHeaders | ResponseBody,

/// <summary>
/// Flag for logging both the HTTP Request and Response.
Expand Down
131 changes: 131 additions & 0 deletions src/Middleware/HttpLogging/src/HttpLoggingInterceptorContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.HttpLogging;

/// <summary>
/// The context used for <see cref="IHttpLoggingInterceptor"/>.
/// </summary>
/// <remarks>
/// Settings will be pre-initialized with the relevant values from <see cref="HttpLoggingOptions" /> and updated with endpoint specific
/// values from <see cref="HttpLoggingAttribute"/> or
/// <see cref="HttpLoggingEndpointConventionBuilderExtensions.WithHttpLogging{TBuilder}(TBuilder, HttpLoggingFields, int?, int?)" />.
/// All settings can be modified per request. All settings will carry over from
/// <see cref="IHttpLoggingInterceptor.OnRequestAsync(HttpLoggingInterceptorContext)"/>
/// to <see cref="IHttpLoggingInterceptor.OnResponseAsync(HttpLoggingInterceptorContext)"/> except the <see cref="Parameters"/>
/// which are cleared after logging the request.
/// </remarks>
public sealed class HttpLoggingInterceptorContext
{
private HttpContext? _httpContext;

/// <summary>
/// The request context.
/// </summary>
/// <remarks>
/// This property should not be set by user code except for testing purposes.
/// </remarks>
public HttpContext HttpContext
{
get => _httpContext ?? throw new InvalidOperationException("HttpContext was not initialized");
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
// Public for 3rd party testing of interceptors.
// We'd make this a required constructor/init parameter but ObjectPool requires a parameterless constructor.
set => _httpContext = value ?? throw new ArgumentNullException(nameof(value));
}

/// <summary>
/// What parts of the request and response to log.
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public HttpLoggingFields LoggingFields { get; set; }

/// <summary>
/// Gets or sets the maximum number of bytes of the request body to log.
/// </summary>
public int RequestBodyLogLimit { get; set; }
Tratcher marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets or sets the maximum number of bytes of the response body to log.
/// </summary>
public int ResponseBodyLogLimit { get; set; }

internal long StartTimestamp { get; set; }
internal TimeProvider TimeProvider { get; set; } = null!;

/// <summary>
/// Data that will be logged as part of the request or response. These values are cleared after logging the request.
/// All other relevant settings will carry over to the response.
/// </summary>
public IList<KeyValuePair<string, object?>> Parameters { get; } = new List<KeyValuePair<string, object?>>();
Tratcher marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Adds data that will be logged as part of the request or response.
/// </summary>
/// <param name="key">The parameter name.</param>
/// <param name="value">The parameter value.</param>
public void AddParameter(string key, object? value)
{
Parameters.Add(new(key, value));
}

/// <summary>
/// Adds the given fields to what's currently enabled in <see cref="LoggingFields"/>.
/// </summary>
/// <param name="fields"></param>
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
public void Enable(HttpLoggingFields fields)
{
LoggingFields |= fields;
}

/// <summary>
/// Checks if any of the given fields are currently enabled in <see cref="LoggingFields"/>.
/// </summary>
public bool IsAnyEnabled(HttpLoggingFields fields)
BrennanConroy marked this conversation as resolved.
Show resolved Hide resolved
{
return (LoggingFields & fields) != HttpLoggingFields.None;
}

/// <summary>
/// Removes the given fields from what's currently enabled in <see cref="LoggingFields"/>.
/// </summary>
/// <param name="fields"></param>
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
public void Disable(HttpLoggingFields fields)
{
LoggingFields &= ~fields;
}

/// <summary>
/// Checks if any of the given fields are currently enabled in <see cref="LoggingFields"/>
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
/// and disables them so that a custom log value can be provided instead.
/// </summary>
/// <param name="fields">One or more field flag to check.</param>
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
/// <returns><see langword="true" /> if any of the fields were previously enabled.</returns>
public bool TryOverride(HttpLoggingFields fields)
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
{
if (IsAnyEnabled(fields))
{
Disable(fields);
return true;
}

return false;
}

internal void Reset()
{
_httpContext = null;
LoggingFields = HttpLoggingFields.None;
RequestBodyLogLimit = 0;
ResponseBodyLogLimit = 0;
StartTimestamp = 0;
TimeProvider = null!;
Parameters.Clear();
}

internal double GetDuration()
{
return TimeProvider.GetElapsedTime(StartTimestamp).TotalMilliseconds;
}
}
Loading