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

#22: catch all downstream errors and log request body #23

Merged
Merged
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
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,16 @@ public class BodyLoggerOptions
/// Controls storage of client IP addresses https://learn.microsoft.com/en-us/azure/azure-monitor/app/ip-collection?tabs=net
/// </summary>
public bool DisableIpMasking { get; set; } = false;

/// <summary>
/// Controls if the middleware should catch and rethrow exceptions to allow logging of request bodies
/// even if downstream middlewares or handlers throw.
/// </summary>
/// <remarks>
/// In some edge cases this might interfere with custom exception handlers or other middlewares catching exceptions.
/// If you enable this feature, make sure that the body logger middleware is registered as early as possible
/// on host creation.
/// </remarks>
public bool EnableBodyLoggingOnExceptions { get; set; } = false;
}
```
24 changes: 22 additions & 2 deletions src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,28 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next)
_bodyReader.PrepareResponseBodyReading(context);
}

// hand over to the next middleware and wait for the call to return
await next(context);
if (_options.EnableBodyLoggingOnExceptions)
{
// hand over to the next middleware and wait for the call to return
try
{
await next(context);
}
catch (Exception)
{
if (_options.HttpVerbs.Contains(context.Request.Method))
{
_telemetryWriter.Write(context, _options.RequestBodyPropertyKey,
_sensitiveDataFilter.RemoveSensitiveData(requestBody));
}

throw;
}
}
else
{
await next(context);
}

if (_options.HttpVerbs.Contains(context.Request.Method))
{
Expand Down
11 changes: 11 additions & 0 deletions src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ public BodyLoggerOptions()
/// </summary>
public bool DisableIpMasking { get; set; } = false;

/// <summary>
/// Controls if the middleware should catch and rethrow exceptions to allow logging of request bodies
/// even if downstream middlewares or handlers throw.
/// </summary>
/// <remarks>
/// In some edge cases this might interfere with custom exception handlers or other middlewares catching exceptions.
/// If you enable this feature, make sure that the body logger middleware is registered as early as possible
/// on host creation.
/// </remarks>
public bool EnableBodyLoggingOnExceptions { get; set; } = false;

public List<string> PropertyNamesWithSensitiveData { get; set; } = new List<string>()
{
"password",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,97 @@ public void BodyLoggerMiddleware_Should_Throw_If_Ctor_Params_Null()
// Assert
action.Should().Throw<NullReferenceException>();
}

[Fact]
public async void BodyLoggerMiddleware_Should_Not_LogRequestBody_If_Downstream_Exception_And_Disabled()
{
// Arrange
var telemetryWriter = new Mock<ITelemetryWriter>();

using var host = await new HostBuilder()
.ConfigureWebHost(webBuilder =>
{
webBuilder
.UseTestServer()
.ConfigureServices(services =>
{
services.AddTransient<IBodyReader, BodyReader>();
services.AddTransient<ISensitiveDataFilter, SensitiveDataFilter>();
services.AddSingleton(telemetryWriter.Object);
services.AddOptions<BodyLoggerOptions>().Configure(options =>
{
options.EnableBodyLoggingOnExceptions = false;
});
services.AddTransient<BodyLoggerMiddleware>();
})
.Configure(app =>
{
app.UseMiddleware<BodyLoggerMiddleware>();
app.Run( _ => throw new Exception("downstream exception"));
});
})
.StartAsync();

// Act
try
{
await host.GetTestClient().PostAsync("/", new StringContent("Hello from client"));
}
catch (Exception)
{
//ignore errors thrown by test client
}

// Assert
telemetryWriter.VerifyNoOtherCalls();

}

[Fact]
public async void BodyLoggerMiddleware_Should_LogRequestBody_If_Downstream_Exception_And_Enabled()
{
// Arrange
var telemetryWriter = new Mock<ITelemetryWriter>();

using var host = await new HostBuilder()
.ConfigureWebHost(webBuilder =>
{
webBuilder
.UseTestServer()
.ConfigureServices(services =>
{
services.AddTransient<IBodyReader, BodyReader>();
services.AddTransient<ISensitiveDataFilter, SensitiveDataFilter>();
services.AddSingleton(telemetryWriter.Object);
services.AddOptions<BodyLoggerOptions>().Configure(options =>
{
options.EnableBodyLoggingOnExceptions = true;
});
services.AddTransient<BodyLoggerMiddleware>();
})
.Configure(app =>
{
app.UseMiddleware<BodyLoggerMiddleware>();
app.Run( _ => throw new Exception("downstream exception"));
});
})
.StartAsync();

// Act
try
{
await host.GetTestClient().PostAsync("/", new StringContent("Hello from client"));
}
catch (Exception)
{
//ignore errors thrown by test client
}

// Assert
telemetryWriter.Verify(x => x.Write(It.IsAny<HttpContext>(), "RequestBody", "Hello from client"), Times.Once);

}


[Fact]
public async void BodyLoggerMiddleware_Should_Send_Data_To_AppInsights()
Expand Down
13 changes: 6 additions & 7 deletions test/ManualTests/Controllers/TestController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ public ActionResult<string> MaxBytesTest([FromForm] string data)
{
return Ok(data);
}

[HttpPost("throw")]
public ActionResult<string> ThrowException([FromBody] TestData data)
{
throw new InvalidOperationException("Test exception!");
}
}
}

public class TestData
{
public string Name { get; set; }
public string Blog { get; set; }
public string Topics { get; set; }
}
2 changes: 1 addition & 1 deletion test/ManualTests/ManualTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<Nullable>enable</Nullable>
<Nullable>disable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>

Expand Down
8 changes: 8 additions & 0 deletions test/ManualTests/Model/TestData.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
public class TestData
{
public string Name { get; set; }

public string Blog { get; set; }

public string Topics { get; set; }
}
4 changes: 3 additions & 1 deletion test/ManualTests/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@

app.Run();

void ConfigureConfiguration(ConfigurationManager configuration) {}
void ConfigureConfiguration(ConfigurationManager configuration) { }
void ConfigureServices(IServiceCollection services)
{
services.AddApplicationInsightsTelemetry();
services.AddAppInsightsHttpBodyLogging(o =>
{
o.HttpCodes.Add(StatusCodes.Status200OK);
o.HttpCodes.Add(StatusCodes.Status204NoContent);
o.DisableIpMasking = true;
o.EnableBodyLoggingOnExceptions = true;
});
services.AddControllers();
services.AddEndpointsApiExplorer();
Expand Down
5 changes: 4 additions & 1 deletion test/ManualTests/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,8 @@
"Microsoft.AspNetCore": "Warning"
}
},
"AllowedHosts": "*"
"AllowedHosts": "*",
"ApplicationInsights": {
"ConnectionString": ""
}
}
Loading