Skip to content

Commit 43d331a

Browse files
committed
Simplify usage of logging in transports
1 parent 75371bf commit 43d331a

File tree

9 files changed

+104
-121
lines changed

9 files changed

+104
-121
lines changed

src/Servers/Kestrel/Transport.Quic/src/Internal/IQuicTrace.cs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using Microsoft.AspNetCore.Connections;
56
using Microsoft.Extensions.Logging;
67

78
namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Experimental.Quic.Internal
89
{
910
internal interface IQuicTrace : ILogger
1011
{
11-
void AcceptedConnection(string connectionId);
12-
void AcceptedStream(string streamId);
13-
void ConnectionError(string connectionId, Exception ex);
14-
void StreamError(string streamId, Exception ex);
15-
void StreamPause(string streamId);
16-
void StreamResume(string streamId);
17-
void StreamShutdownWrite(string streamId, string reason);
18-
void StreamAbort(string streamId, string reason);
12+
void AcceptedConnection(BaseConnectionContext connection);
13+
void AcceptedStream(QuicStreamContext streamContext);
14+
void ConnectionError(BaseConnectionContext connection, Exception ex);
15+
void StreamError(QuicStreamContext streamContext, Exception ex);
16+
void StreamPause(QuicStreamContext streamContext);
17+
void StreamResume(QuicStreamContext streamContext);
18+
void StreamShutdownWrite(QuicStreamContext streamContext, string reason);
19+
void StreamAbort(QuicStreamContext streamContext, string reason);
1920
}
2021
}

src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionContext.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,7 @@ public QuicConnectionContext(QuicConnection connection, QuicTransportContext con
3232
Features.Set<ITlsConnectionFeature>(new FakeTlsConnectionFeature());
3333
Features.Set<IProtocolErrorCodeFeature>(this);
3434

35-
if (_log.IsEnabled(LogLevel.Debug))
36-
{
37-
_log.AcceptedConnection(ConnectionId);
38-
}
35+
_log.AcceptedConnection(this);
3936
}
4037

4138
public ValueTask<ConnectionContext> StartUnidirectionalStreamAsync()

src/Servers/Kestrel/Transport.Quic/src/Internal/QuicStreamContext.cs

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,7 @@ private async Task DoReceive()
131131
{
132132
// This is unexpected.
133133
error = ex;
134-
if (_log.IsEnabled(LogLevel.Debug))
135-
{
136-
_log.StreamError(ConnectionId, error);
137-
}
134+
_log.StreamError(this, error);
138135
}
139136
finally
140137
{
@@ -167,16 +164,16 @@ private async Task ProcessReceives()
167164

168165
var paused = !flushTask.IsCompleted;
169166

170-
if (paused && _log.IsEnabled(LogLevel.Debug))
167+
if (paused)
171168
{
172-
_log.StreamPause(ConnectionId);
169+
_log.StreamPause(this);
173170
}
174171

175172
var result = await flushTask;
176173

177-
if (paused && _log.IsEnabled(LogLevel.Debug))
174+
if (paused)
178175
{
179-
_log.StreamResume(ConnectionId);
176+
_log.StreamResume(this);
180177
}
181178

182179
if (result.IsCompleted || result.IsCanceled)
@@ -237,10 +234,7 @@ private async Task DoSend()
237234
{
238235
shutdownReason = ex;
239236
unexpectedError = ex;
240-
if (_log.IsEnabled(LogLevel.Debug))
241-
{
242-
_log.ConnectionError(ConnectionId, unexpectedError);
243-
}
237+
_log.ConnectionError(this, unexpectedError);
244238
}
245239
finally
246240
{
@@ -297,10 +291,7 @@ public override void Abort(ConnectionAbortedException abortReason)
297291

298292
_aborted = true;
299293

300-
if (_log.IsEnabled(LogLevel.Debug))
301-
{
302-
_log.StreamAbort(ConnectionId, abortReason.Message);
303-
}
294+
_log.StreamAbort(this, abortReason.Message);
304295

305296
lock (_shutdownLock)
306297
{
@@ -320,11 +311,8 @@ private async ValueTask ShutdownWrite(Exception? shutdownReason)
320311
{
321312
// TODO: Exception is always allocated. Consider only allocating if receive hasn't completed.
322313
_shutdownReason = shutdownReason ?? new ConnectionAbortedException("The Quic transport's send loop completed gracefully.");
314+
_log.StreamShutdownWrite(this, _shutdownReason.Message);
323315

324-
if (_log.IsEnabled(LogLevel.Debug))
325-
{
326-
_log.StreamShutdownWrite(ConnectionId, _shutdownReason.Message);
327-
}
328316
_stream.Shutdown();
329317
}
330318

src/Servers/Kestrel/Transport.Quic/src/Internal/QuicTrace.cs

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using Microsoft.AspNetCore.Connections;
56
using Microsoft.Extensions.Logging;
67

78
namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Experimental.Quic.Internal
@@ -11,7 +12,7 @@ internal class QuicTrace : IQuicTrace
1112
private static readonly Action<ILogger, string, Exception?> _acceptedConnection =
1213
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(1, "AcceptedConnection"), @"Connection id ""{ConnectionId}"" accepted.", skipEnabledCheck: true);
1314
private static readonly Action<ILogger, string, Exception?> _acceptedStream =
14-
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(2, "AcceptedStream"), @"Stream id ""{ConnectionId}"" accepted.");
15+
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(2, "AcceptedStream"), @"Stream id ""{ConnectionId}"" accepted.", skipEnabledCheck: true);
1516
private static readonly Action<ILogger, string, Exception?> _connectionError =
1617
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(3, "ConnectionError"), @"Connection id ""{ConnectionId}"" unexpected error.", skipEnabledCheck: true);
1718
private static readonly Action<ILogger, string, Exception?> _streamError =
@@ -39,44 +40,60 @@ public QuicTrace(ILogger logger)
3940
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
4041
=> _logger.Log(logLevel, eventId, state, exception, formatter);
4142

42-
public void AcceptedConnection(string connectionId)
43+
public void AcceptedConnection(BaseConnectionContext connection)
4344
{
44-
_acceptedConnection(_logger, connectionId, null);
45+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
46+
47+
_acceptedConnection(_logger, connection.ConnectionId, null);
4548
}
4649

47-
public void AcceptedStream(string streamId)
50+
public void AcceptedStream(QuicStreamContext streamContext)
4851
{
49-
_acceptedStream(_logger, streamId, null);
52+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
53+
54+
_acceptedStream(_logger, streamContext.ConnectionId, null);
5055
}
5156

52-
public void ConnectionError(string connectionId, Exception ex)
57+
public void ConnectionError(BaseConnectionContext connection, Exception ex)
5358
{
54-
_connectionError(_logger, connectionId, ex);
59+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
60+
61+
_connectionError(_logger, connection.ConnectionId, ex);
5562
}
5663

57-
public void StreamError(string streamId, Exception ex)
64+
public void StreamError(QuicStreamContext streamContext, Exception ex)
5865
{
59-
_streamError(_logger, streamId, ex);
66+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
67+
68+
_streamError(_logger, streamContext.ConnectionId, ex);
6069
}
6170

62-
public void StreamPause(string streamId)
71+
public void StreamPause(QuicStreamContext streamContext)
6372
{
64-
_streamPause(_logger, streamId, null);
73+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
74+
75+
_streamPause(_logger, streamContext.ConnectionId, null);
6576
}
6677

67-
public void StreamResume(string streamId)
78+
public void StreamResume(QuicStreamContext streamContext)
6879
{
69-
_streamResume(_logger, streamId, null);
80+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
81+
82+
_streamResume(_logger, streamContext.ConnectionId, null);
7083
}
7184

72-
public void StreamShutdownWrite(string streamId, string reason)
85+
public void StreamShutdownWrite(QuicStreamContext streamContext, string reason)
7386
{
74-
_streamShutdownWrite(_logger, streamId, reason, null);
87+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
88+
89+
_streamShutdownWrite(_logger, streamContext.ConnectionId, reason, null);
7590
}
7691

77-
public void StreamAbort(string streamId, string reason)
92+
public void StreamAbort(QuicStreamContext streamContext, string reason)
7893
{
79-
_streamAborted(_logger, streamId, reason, null);
94+
if (!_logger.IsEnabled(LogLevel.Debug)) return;
95+
96+
_streamAborted(_logger, streamContext.ConnectionId, reason, null);
8097
}
8198
}
8299
}

src/Servers/Kestrel/Transport.Sockets/src/Internal/ISocketsTrace.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
88
{
99
internal interface ISocketsTrace : ILogger
1010
{
11-
void ConnectionReadFin(string connectionId);
11+
void ConnectionReadFin(SocketConnection connection);
1212

13-
void ConnectionWriteFin(string connectionId, string reason);
13+
void ConnectionWriteFin(SocketConnection connection, string reason);
1414

15-
void ConnectionError(string connectionId, Exception ex);
15+
void ConnectionError(SocketConnection connection, Exception ex);
1616

1717
void ConnectionReset(string connectionId);
18+
void ConnectionReset(SocketConnection connection);
1819

19-
void ConnectionPause(string connectionId);
20+
void ConnectionPause(SocketConnection connection);
2021

21-
void ConnectionResume(string connectionId);
22+
void ConnectionResume(SocketConnection connection);
2223
}
2324
}

src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,7 @@ private async Task DoReceive()
156156
if (bytesReceived == 0)
157157
{
158158
// FIN
159-
if (_trace.IsEnabled(LogLevel.Debug))
160-
{
161-
_trace.ConnectionReadFin(ConnectionId);
162-
}
159+
_trace.ConnectionReadFin(this);
163160
break;
164161
}
165162

@@ -169,16 +166,16 @@ private async Task DoReceive()
169166

170167
var paused = !flushTask.IsCompleted;
171168

172-
if (paused && _trace.IsEnabled(LogLevel.Debug))
169+
if (paused)
173170
{
174-
_trace.ConnectionPause(ConnectionId);
171+
_trace.ConnectionPause(this);
175172
}
176173

177174
var result = await flushTask;
178175

179-
if (paused && _trace.IsEnabled(LogLevel.Debug))
176+
if (paused)
180177
{
181-
_trace.ConnectionResume(ConnectionId);
178+
_trace.ConnectionResume(this);
182179
}
183180

184181
if (result.IsCompleted || result.IsCanceled)
@@ -197,7 +194,7 @@ private async Task DoReceive()
197194
// Both logs will have the same ConnectionId. I don't think it's worthwhile to lock just to avoid this.
198195
if (!_socketDisposed)
199196
{
200-
_trace.ConnectionReset(ConnectionId);
197+
_trace.ConnectionReset(this);
201198
}
202199
}
203200
catch (Exception ex)
@@ -207,20 +204,17 @@ private async Task DoReceive()
207204
// This exception should always be ignored because _shutdownReason should be set.
208205
error = ex;
209206

210-
if (!_socketDisposed && _trace.IsEnabled(LogLevel.Debug))
207+
if (!_socketDisposed)
211208
{
212209
// This is unexpected if the socket hasn't been disposed yet.
213-
_trace.ConnectionError(ConnectionId, error);
210+
_trace.ConnectionError(this, error);
214211
}
215212
}
216213
catch (Exception ex)
217214
{
218215
// This is unexpected.
219216
error = ex;
220-
if (_trace.IsEnabled(LogLevel.Debug))
221-
{
222-
_trace.ConnectionError(ConnectionId, error);
223-
}
217+
_trace.ConnectionError(this, error);
224218
}
225219
finally
226220
{
@@ -271,10 +265,7 @@ private async Task DoSend()
271265
catch (SocketException ex) when (IsConnectionResetError(ex.SocketErrorCode))
272266
{
273267
shutdownReason = new ConnectionResetException(ex.Message, ex);
274-
if (_trace.IsEnabled(LogLevel.Debug))
275-
{
276-
_trace.ConnectionReset(ConnectionId);
277-
}
268+
_trace.ConnectionReset(this);
278269
}
279270
catch (Exception ex)
280271
when ((ex is SocketException socketEx && IsConnectionAbortError(socketEx.SocketErrorCode)) ||
@@ -287,10 +278,7 @@ private async Task DoSend()
287278
{
288279
shutdownReason = ex;
289280
unexpectedError = ex;
290-
if (_trace.IsEnabled(LogLevel.Debug))
291-
{
292-
_trace.ConnectionError(ConnectionId, unexpectedError);
293-
}
281+
_trace.ConnectionError(this, unexpectedError);
294282
}
295283
finally
296284
{
@@ -342,11 +330,7 @@ private void Shutdown(Exception? shutdownReason)
342330
// ever observe the nondescript ConnectionAbortedException except for connection middleware attempting
343331
// to half close the connection which is currently unsupported.
344332
_shutdownReason = shutdownReason ?? new ConnectionAbortedException("The Socket transport's send loop completed gracefully.");
345-
346-
if (_trace.IsEnabled(LogLevel.Debug))
347-
{
348-
_trace.ConnectionWriteFin(ConnectionId, _shutdownReason.Message);
349-
}
333+
_trace.ConnectionWriteFin(this, _shutdownReason.Message);
350334

351335
try
352336
{

0 commit comments

Comments
 (0)