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

Improve Kestrel connection metrics with error reasons #55565

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ internal class BaseHttpConnectionContext
IFeatureCollection connectionFeatures,
MemoryPool<byte> memoryPool,
IPEndPoint? localEndPoint,
IPEndPoint? remoteEndPoint)
IPEndPoint? remoteEndPoint,
ConnectionMetricsContext metricsContext)
{
ConnectionId = connectionId;
Protocols = protocols;
Expand All @@ -31,6 +32,7 @@ internal class BaseHttpConnectionContext
MemoryPool = memoryPool;
LocalEndPoint = localEndPoint;
RemoteEndPoint = remoteEndPoint;
MetricsContext = metricsContext;
}

public string ConnectionId { get; set; }
Expand All @@ -42,6 +44,7 @@ internal class BaseHttpConnectionContext
public MemoryPool<byte> MemoryPool { get; }
public IPEndPoint? LocalEndPoint { get; }
public IPEndPoint? RemoteEndPoint { get; }
public ConnectionMetricsContext MetricsContext { get; }

public ITimeoutControl TimeoutControl { get; set; } = default!; // Always set by HttpConnection
public ExecutionContext? InitialExecutionContext { get; set; }
Expand Down
92 changes: 79 additions & 13 deletions src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,16 @@ public Http1Connection(HttpConnectionContext context)
_context.ServiceContext.Log,
_context.TimeoutControl,
minResponseDataRateFeature: this,
MetricsContext,
outputAborter: this);

Input = _context.Transport.Input;
Output = _http1Output;
MemoryPool = _context.MemoryPool;
}

public ConnectionMetricsContext MetricsContext => _context.MetricsContext;

public PipeReader Input { get; }

public bool RequestTimedOut => _requestTimedOut;
Expand All @@ -82,7 +85,7 @@ protected override void OnRequestProcessingEnded()
if (IsUpgraded)
{
KestrelEventSource.Log.RequestUpgradedStop(this);
ServiceContext.Metrics.RequestUpgradedStop(_context.MetricsContext);
ServiceContext.Metrics.RequestUpgradedStop(MetricsContext);

ServiceContext.ConnectionManager.UpgradedConnectionCount.ReleaseOne();
}
Expand All @@ -98,39 +101,41 @@ protected override void OnRequestProcessingEnded()
void IRequestProcessor.OnInputOrOutputCompleted()
{
// Closed gracefully.
_http1Output.Abort(ServerOptions.FinOnError ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!);
_http1Output.Abort(ServerOptions.FinOnError ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!, ConnectionEndReason.TransportCompleted);
CancelRequestAbortedToken();
}

void IHttpOutputAborter.OnInputOrOutputCompleted()
{
_http1Output.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient));
_http1Output.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient), ConnectionEndReason.TransportCompleted);
JamesNK marked this conversation as resolved.
Show resolved Hide resolved
CancelRequestAbortedToken();
}

/// <summary>
/// Immediately kill the connection and poison the request body stream with an error.
/// </summary>
public void Abort(ConnectionAbortedException abortReason)
public void Abort(ConnectionAbortedException abortReason, ConnectionEndReason reason)
{
_http1Output.Abort(abortReason);
_http1Output.Abort(abortReason, reason);
CancelRequestAbortedToken();
PoisonBody(abortReason);
}

protected override void ApplicationAbort()
{
Log.ApplicationAbortedConnection(ConnectionId, TraceIdentifier);
Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication));
Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication), ConnectionEndReason.AbortedByApp);
}

/// <summary>
/// Stops the request processing loop between requests.
/// Called on all active connections when the server wants to initiate a shutdown
/// and after a keep-alive timeout.
/// </summary>
public void StopProcessingNextRequest()
public void StopProcessingNextRequest(ConnectionEndReason reason)
{
KestrelMetrics.AddConnectionEndReason(MetricsContext, reason);

_keepAlive = false;
Input.CancelPendingRead();
}
Expand All @@ -142,12 +147,16 @@ public void SendTimeoutResponse()
}

public void HandleRequestHeadersTimeout()
=> SendTimeoutResponse();
{
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.RequestHeadersTimeout);
SendTimeoutResponse();
}

public void HandleReadDataRateTimeout()
{
Debug.Assert(MinRequestBodyDataRate != null);

KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.MinRequestBodyDataRate);
Log.RequestBodyMinimumDataRateNotSatisfied(ConnectionId, TraceIdentifier, MinRequestBodyDataRate.BytesPerSecond);
SendTimeoutResponse();
}
Expand Down Expand Up @@ -701,17 +710,22 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
}
catch (InvalidOperationException) when (_requestProcessingStatus == RequestProcessingStatus.ParsingHeaders)
{
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.InvalidRequestHeaders);
KestrelBadHttpRequestException.Throw(RequestRejectionReason.MalformedRequestInvalidHeaders);
throw;
}
#pragma warning disable CS0618 // Type or member is obsolete
catch (BadHttpRequestException ex)
{
DetectHttp2Preface(result.Buffer, ex);

OnBadRequest(result.Buffer, ex);
throw;
}
#pragma warning restore CS0618 // Type or member is obsolete
catch (Exception)
{
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.OtherError);
throw;
}
finally
{
Input.AdvanceTo(reader.Position, isConsumed ? reader.Position : result.Buffer.End);
Expand All @@ -725,9 +739,11 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
endConnection = true;
return true;
case RequestProcessingStatus.ParsingRequestLine:
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.InvalidRequestHeaders);
KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestLine);
break;
case RequestProcessingStatus.ParsingHeaders:
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.InvalidRequestHeaders);
KestrelBadHttpRequestException.Throw(RequestRejectionReason.MalformedRequestInvalidHeaders);
break;
}
Expand All @@ -743,6 +759,7 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
{
// In this case, there is an ongoing request but the start line/header parsing has timed out, so send
// a 408 response.
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.RequestHeadersTimeout);
KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestHeadersTimeout);
}

Expand All @@ -759,8 +776,58 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
}

#pragma warning disable CS0618 // Type or member is obsolete
private void DetectHttp2Preface(ReadOnlySequence<byte> requestData, BadHttpRequestException ex)
private void OnBadRequest(ReadOnlySequence<byte> requestData, BadHttpRequestException ex)
#pragma warning restore CS0618 // Type or member is obsolete
{
var reason = ex.Reason;

// Some code shared between HTTP versions throws errors. For example, HttpRequestHeaders collection
// throws when an invalid content length is set.
// Only want to set a reasons for HTTP/1.1 connection, so set end reason by catching the exception here.
switch (reason)
Copy link
Member Author

Choose a reason for hiding this comment

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

@halter73 @Tratcher It would be good to get your thoughts on whether new connection end reasons should be added based on available RequestRejectionReason values thrown when parsing a 1.1 request.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe for the more common ones that aren't just buggy clients like
UnexpectedEndOfRequestContent
RequestBodyTooLarge
RequestBodyTimeout

{
case RequestRejectionReason.UnrecognizedHTTPVersion:
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.InvalidHttpVersion);
DetectHttp2Preface(requestData);
break;
case RequestRejectionReason.InvalidRequestLine:
case RequestRejectionReason.RequestLineTooLong:
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.InvalidRequestLine);
break;
case RequestRejectionReason.InvalidRequestHeadersNoCRLF:
case RequestRejectionReason.InvalidRequestHeader:
case RequestRejectionReason.InvalidContentLength:
case RequestRejectionReason.HeadersExceedMaxTotalSize:
case RequestRejectionReason.TooManyHeaders:
case RequestRejectionReason.MultipleContentLengths:
case RequestRejectionReason.MalformedRequestInvalidHeaders:
case RequestRejectionReason.InvalidRequestTarget:
case RequestRejectionReason.InvalidCharactersInHeaderName:
case RequestRejectionReason.LengthRequiredHttp10:
case RequestRejectionReason.OptionsMethodRequired:
case RequestRejectionReason.ConnectMethodRequired:
case RequestRejectionReason.MissingHostHeader:
case RequestRejectionReason.MultipleHostHeaders:
case RequestRejectionReason.InvalidHostHeader:
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.InvalidRequestHeaders);
break;
case RequestRejectionReason.TlsOverHttpError:
case RequestRejectionReason.UnexpectedEndOfRequestContent:
case RequestRejectionReason.BadChunkSuffix:
case RequestRejectionReason.BadChunkSizeData:
case RequestRejectionReason.ChunkedRequestIncomplete:
case RequestRejectionReason.RequestBodyTooLarge:
case RequestRejectionReason.RequestHeadersTimeout:
case RequestRejectionReason.RequestBodyTimeout:
case RequestRejectionReason.FinalTransferCodingNotChunked:
case RequestRejectionReason.RequestBodyExceedsContentLength:
default:
KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.OtherError);
break;
}
}

private void DetectHttp2Preface(ReadOnlySequence<byte> requestData)
{
const int PrefaceLineLength = 16;

Expand All @@ -770,8 +837,7 @@ private void DetectHttp2Preface(ReadOnlySequence<byte> requestData, BadHttpReque
{
// If there is an unrecognized HTTP version, it is the first request on the connection, and the request line
// bytes matches the HTTP/2 preface request line bytes then log and return a HTTP/2 GOAWAY frame.
if (ex.Reason == RequestRejectionReason.UnrecognizedHTTPVersion
&& _requestCount == 1
if (_requestCount == 1
&& requestData.Length >= PrefaceLineLength)
{
var clientPrefaceRequestLine = Http2.Http2Connection.ClientPreface.Slice(0, PrefaceLineLength);
Expand Down
10 changes: 4 additions & 6 deletions src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,10 @@ protected override Task OnConsumeAsync()
}
catch (InvalidOperationException ex)
{
var connectionAbortedException = new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication, ex);
_context.ReportApplicationError(connectionAbortedException);
Log.RequestBodyDrainBodyReaderInvalidState(_context.ConnectionIdFeature, _context.TraceIdentifier, ex);

// Have to abort the connection because we can't finish draining the request
_context.StopProcessingNextRequest();
_context.StopProcessingNextRequest(ConnectionEndReason.BodyReaderInvalidState);
return Task.CompletedTask;
}

Expand Down Expand Up @@ -104,11 +103,10 @@ protected async Task OnConsumeAsyncAwaited()
}
catch (InvalidOperationException ex)
{
var connectionAbortedException = new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication, ex);
_context.ReportApplicationError(connectionAbortedException);
Log.RequestBodyDrainBodyReaderInvalidState(_context.ConnectionIdFeature, _context.TraceIdentifier, ex);

// Have to abort the connection because we can't finish draining the request
_context.StopProcessingNextRequest();
_context.StopProcessingNextRequest(ConnectionEndReason.BodyReaderInvalidState);
}
finally
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ internal class Http1OutputProducer : IHttpOutputProducer, IDisposable
private readonly MemoryPool<byte> _memoryPool;
private readonly KestrelTrace _log;
private readonly IHttpMinResponseDataRateFeature _minResponseDataRateFeature;
private readonly ConnectionMetricsContext _connectionMetricsContext;
private readonly IHttpOutputAborter _outputAborter;
private readonly TimingPipeFlusher _flusher;

Expand Down Expand Up @@ -74,6 +75,7 @@ internal class Http1OutputProducer : IHttpOutputProducer, IDisposable
KestrelTrace log,
ITimeoutControl timeoutControl,
IHttpMinResponseDataRateFeature minResponseDataRateFeature,
ConnectionMetricsContext connectionMetricsContext,
IHttpOutputAborter outputAborter)
{
// Allow appending more data to the PipeWriter when a flush is pending.
Expand All @@ -83,6 +85,7 @@ internal class Http1OutputProducer : IHttpOutputProducer, IDisposable
_memoryPool = memoryPool;
_log = log;
_minResponseDataRateFeature = minResponseDataRateFeature;
_connectionMetricsContext = connectionMetricsContext;
_outputAborter = outputAborter;

_flusher = new TimingPipeFlusher(timeoutControl, log);
Expand Down Expand Up @@ -444,7 +447,7 @@ private void CompletePipe()
}
}

public void Abort(ConnectionAbortedException error)
public void Abort(ConnectionAbortedException error, ConnectionEndReason reason)
{
// Abort can be called after Dispose if there's a flush timeout.
// It's important to still call _lifetimeFeature.Abort() in this case.
Expand All @@ -455,6 +458,8 @@ public void Abort(ConnectionAbortedException error)
return;
}

KestrelMetrics.AddConnectionEndReason(_connectionMetricsContext, reason);

_aborted = true;
_connectionContext.Abort(error);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;

internal interface IHttpOutputAborter
{
void Abort(ConnectionAbortedException abortReason);
void Abort(ConnectionAbortedException abortReason, ConnectionEndReason reason);
void OnInputOrOutputCompleted();
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public bool TryAdvance(int bytes)
// flow-control window at the time of the abort.
if (bytes > _flow.Available)
{
throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorFlowControlWindowExceeded, Http2ErrorCode.FLOW_CONTROL_ERROR);
throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorFlowControlWindowExceeded, Http2ErrorCode.FLOW_CONTROL_ERROR, ConnectionEndReason.FlowControlWindowExceeded);
}

if (_flow.IsAborted)
Expand Down