Skip to content

Commit a93e67b

Browse files
update logging (#4188)
1 parent 3f681f7 commit a93e67b

13 files changed

+135
-166
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"services": [
3+
{
4+
"serviceName": "S3",
5+
"type": "patch",
6+
"changeLogMessages": [
7+
"Fix Transfer Utility internal Logger recursive property definition"
8+
]
9+
}
10+
]
11+
}

sdk/src/Services/S3/Custom/Transfer/Internal/BufferedMultipartStream.cs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,7 @@ internal class BufferedMultipartStream : Stream
4646
private DownloadDiscoveryResult _discoveryResult;
4747
private long _totalBytesRead = 0;
4848

49-
private Logger Logger
50-
{
51-
get { return Logger.GetLogger(typeof(TransferUtility)); }
52-
}
49+
private readonly Logger _logger = Logger.GetLogger(typeof(BufferedMultipartStream));
5350

5451
/// <summary>
5552
/// Gets the <see cref="DownloadDiscoveryResult"/> containing metadata from the initial GetObject response.
@@ -113,12 +110,12 @@ public async Task InitializeAsync(CancellationToken cancellationToken)
113110
if (_initialized)
114111
throw new InvalidOperationException("Stream has already been initialized");
115112

116-
Logger.DebugFormat("BufferedMultipartStream: Starting initialization");
113+
_logger.DebugFormat("BufferedMultipartStream: Starting initialization");
117114

118115
_discoveryResult = await _downloadCoordinator.DiscoverDownloadStrategyAsync(cancellationToken)
119116
.ConfigureAwait(false);
120117

121-
Logger.DebugFormat("BufferedMultipartStream: Discovery completed - ObjectSize={0}, TotalParts={1}, IsSinglePart={2}",
118+
_logger.DebugFormat("BufferedMultipartStream: Discovery completed - ObjectSize={0}, TotalParts={1}, IsSinglePart={2}",
122119
_discoveryResult.ObjectSize,
123120
_discoveryResult.TotalParts,
124121
_discoveryResult.IsSinglePart);
@@ -127,7 +124,7 @@ await _downloadCoordinator.StartDownloadsAsync(_discoveryResult, null, cancellat
127124
.ConfigureAwait(false);
128125

129126
_initialized = true;
130-
Logger.DebugFormat("BufferedMultipartStream: Initialization completed successfully");
127+
_logger.DebugFormat("BufferedMultipartStream: Initialization completed successfully");
131128
}
132129

133130
/// <summary>
@@ -168,7 +165,7 @@ public override async Task<int> ReadAsync(byte[] buffer, int offset, int count,
168165
throw new ArgumentException("Offset and count exceed buffer bounds");
169166

170167
var currentPosition = Interlocked.Read(ref _totalBytesRead);
171-
Logger.DebugFormat("BufferedMultipartStream: ReadAsync called - Position={0}, RequestedBytes={1}",
168+
_logger.DebugFormat("BufferedMultipartStream: ReadAsync called - Position={0}, RequestedBytes={1}",
172169
currentPosition, count);
173170

174171
var bytesRead = await _partBufferManager.ReadAsync(buffer, offset, count, cancellationToken)
@@ -178,12 +175,12 @@ public override async Task<int> ReadAsync(byte[] buffer, int offset, int count,
178175
if (bytesRead > 0)
179176
{
180177
Interlocked.Add(ref _totalBytesRead, bytesRead);
181-
Logger.DebugFormat("BufferedMultipartStream: ReadAsync completed - BytesRead={0}, NewPosition={1}",
178+
_logger.DebugFormat("BufferedMultipartStream: ReadAsync completed - BytesRead={0}, NewPosition={1}",
182179
bytesRead, currentPosition + bytesRead);
183180
}
184181
else
185182
{
186-
Logger.DebugFormat("BufferedMultipartStream: ReadAsync returned EOF (0 bytes)");
183+
_logger.DebugFormat("BufferedMultipartStream: ReadAsync returned EOF (0 bytes)");
187184
}
188185

189186
return bytesRead;

sdk/src/Services/S3/Custom/Transfer/Internal/BufferedPartDataHandler.cs

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,7 @@ internal class BufferedPartDataHandler : IPartDataHandler
5555
private readonly IPartBufferManager _partBufferManager;
5656
private readonly BufferedDownloadConfiguration _config;
5757

58-
private Logger Logger
59-
{
60-
get { return Logger.GetLogger(typeof(TransferUtility)); }
61-
}
58+
private readonly Logger _logger = Logger.GetLogger(typeof(BufferedPartDataHandler));
6259

6360
/// <summary>
6461
/// Initializes a new instance of the <see cref="BufferedPartDataHandler"/> class.
@@ -137,7 +134,7 @@ private async Task ProcessStreamingPartAsync(
137134
GetObjectResponse response,
138135
CancellationToken cancellationToken)
139136
{
140-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Matches NextExpectedPartNumber - streaming directly without buffering",
137+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Matches NextExpectedPartNumber - streaming directly without buffering",
141138
partNumber);
142139

143140
StreamingDataSource streamingDataSource = null;
@@ -161,12 +158,12 @@ private async Task ProcessStreamingPartAsync(
161158
// Release capacity immediately since we're not holding anything in memory
162159
_partBufferManager.ReleaseBufferSpace();
163160

164-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] StreamingDataSource added and capacity released",
161+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] StreamingDataSource added and capacity released",
165162
partNumber);
166163
}
167164
catch (Exception ex)
168165
{
169-
Logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process streaming part", partNumber);
166+
_logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process streaming part", partNumber);
170167

171168
// Dispose response if we still own it (constructor failed before taking ownership)
172169
if (ownsResponse)
@@ -206,7 +203,7 @@ private async Task ProcessBufferedPartAsync(
206203
GetObjectResponse response,
207204
CancellationToken cancellationToken)
208205
{
209-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Out of order (NextExpected={1}) - buffering to memory",
206+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Out of order (NextExpected={1}) - buffering to memory",
210207
partNumber, _partBufferManager.NextExpectedPartNumber);
211208

212209
try
@@ -220,18 +217,18 @@ private async Task ProcessBufferedPartAsync(
220217
// Response has been fully read and buffered - dispose it now
221218
response?.Dispose();
222219

223-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Buffered {1} bytes into memory",
220+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Buffered {1} bytes into memory",
224221
partNumber, buffer.Length);
225222

226223
// Add the buffered part to the buffer manager
227224
_partBufferManager.AddBuffer(buffer);
228225

229-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Added to buffer manager (capacity will be released after consumption)",
226+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Added to buffer manager (capacity will be released after consumption)",
230227
partNumber);
231228
}
232229
catch (Exception ex)
233230
{
234-
Logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process buffered part", partNumber);
231+
_logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process buffered part", partNumber);
235232

236233
// We own the response throughout this method, so dispose it on error
237234
response?.Dispose();
@@ -286,7 +283,7 @@ private async Task<StreamPartBuffer> BufferPartFromResponseAsync(
286283
long expectedBytes = response.ContentLength;
287284
int initialBufferSize = (int)expectedBytes;
288285

289-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Allocating buffer of size {1} bytes from ArrayPool",
286+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Allocating buffer of size {1} bytes from ArrayPool",
290287
partNumber, initialBufferSize);
291288

292289
downloadedPart = StreamPartBuffer.Create(partNumber, initialBufferSize);
@@ -299,7 +296,7 @@ private async Task<StreamPartBuffer> BufferPartFromResponseAsync(
299296
// The MemoryStream starts at position 0 and can grow up to initialBufferSize
300297
using (var memoryStream = new MemoryStream(partBuffer, 0, initialBufferSize, writable: true))
301298
{
302-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Reading response stream into buffer",
299+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Reading response stream into buffer",
303300
partNumber);
304301

305302
// Use GetObjectResponse's stream copy logic which includes:
@@ -316,15 +313,15 @@ await response.WriteResponseStreamAsync(
316313

317314
int totalRead = (int)memoryStream.Position;
318315

319-
Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Read {1} bytes from response stream",
316+
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Read {1} bytes from response stream",
320317
partNumber, totalRead);
321318

322319
// Set the length to reflect actual bytes read
323320
downloadedPart.SetLength(totalRead);
324321

325322
if (totalRead != expectedBytes)
326323
{
327-
Logger.Error(null, "BufferedPartDataHandler: [Part {0}] Size mismatch - Expected {1} bytes, read {2} bytes",
324+
_logger.Error(null, "BufferedPartDataHandler: [Part {0}] Size mismatch - Expected {1} bytes, read {2} bytes",
328325
partNumber, expectedBytes, totalRead);
329326
}
330327
}
@@ -333,7 +330,7 @@ await response.WriteResponseStreamAsync(
333330
}
334331
catch (Exception ex)
335332
{
336-
Logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to buffer part from response stream", partNumber);
333+
_logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to buffer part from response stream", partNumber);
337334
// If something goes wrong, StreamPartBuffer.Dispose() will handle cleanup
338335
downloadedPart?.Dispose();
339336
throw;

sdk/src/Services/S3/Custom/Transfer/Internal/FilePartDataHandler.cs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,7 @@ internal class FilePartDataHandler : IPartDataHandler
4242
private string _tempFilePath;
4343
private bool _disposed = false;
4444

45-
private Logger Logger
46-
{
47-
get { return Logger.GetLogger(typeof(TransferUtility)); }
48-
}
45+
private readonly Logger _logger = Logger.GetLogger(typeof(FilePartDataHandler));
4946

5047
/// <summary>
5148
/// Initializes a new instance for file downloads.
@@ -63,7 +60,7 @@ public Task PrepareAsync(DownloadDiscoveryResult discoveryResult, CancellationTo
6360
// Create temporary file once during preparation phase
6461
_tempFilePath = _fileHandler.CreateTemporaryFile(_config.DestinationFilePath);
6562

66-
Logger.DebugFormat("FilePartDataHandler: Created temporary file for download");
63+
_logger.DebugFormat("FilePartDataHandler: Created temporary file for download");
6764

6865
return Task.CompletedTask;
6966
}
@@ -83,20 +80,20 @@ public async Task ProcessPartAsync(
8380
{
8481
try
8582
{
86-
Logger.DebugFormat("FilePartDataHandler: [Part {0}] Starting to process part - ContentLength={1}",
83+
_logger.DebugFormat("FilePartDataHandler: [Part {0}] Starting to process part - ContentLength={1}",
8784
partNumber, response.ContentLength);
8885

8986
// Calculate offset for this part based on ContentRange or part number
9087
long offset = GetPartOffset(response, partNumber);
9188

92-
Logger.DebugFormat("FilePartDataHandler: [Part {0}] Calculated file offset={1}",
89+
_logger.DebugFormat("FilePartDataHandler: [Part {0}] Calculated file offset={1}",
9390
partNumber, offset);
9491

9592
// Write part data to file at the calculated offset
9693
await WritePartToFileAsync(offset, response, cancellationToken)
9794
.ConfigureAwait(false);
9895

99-
Logger.DebugFormat("FilePartDataHandler: [Part {0}] File write completed successfully",
96+
_logger.DebugFormat("FilePartDataHandler: [Part {0}] File write completed successfully",
10097
partNumber);
10198
}
10299
finally
@@ -128,17 +125,17 @@ public void OnDownloadComplete(Exception exception)
128125
if (exception == null)
129126
{
130127
// Success - commit temp file to final destination
131-
Logger.DebugFormat("FilePartDataHandler: Download complete, committing temporary file to destination");
128+
_logger.DebugFormat("FilePartDataHandler: Download complete, committing temporary file to destination");
132129

133130
try
134131
{
135132
_fileHandler.CommitFile(_tempFilePath, _config.DestinationFilePath);
136133

137-
Logger.DebugFormat("FilePartDataHandler: Successfully committed file to destination");
134+
_logger.DebugFormat("FilePartDataHandler: Successfully committed file to destination");
138135
}
139136
catch (Exception commitException)
140137
{
141-
Logger.Error(commitException, "FilePartDataHandler: Failed to commit file to destination");
138+
_logger.Error(commitException, "FilePartDataHandler: Failed to commit file to destination");
142139

143140
// Cleanup on commit failure
144141
_fileHandler.CleanupOnFailure();
@@ -149,7 +146,7 @@ public void OnDownloadComplete(Exception exception)
149146
else
150147
{
151148
// Failure - cleanup temp file
152-
Logger.Error(exception, "FilePartDataHandler: Download failed, cleaning up temporary file");
149+
_logger.Error(exception, "FilePartDataHandler: Download failed, cleaning up temporary file");
153150

154151
_fileHandler.CleanupOnFailure();
155152
}
@@ -202,7 +199,7 @@ private async Task WritePartToFileAsync(
202199
if (string.IsNullOrEmpty(_tempFilePath))
203200
throw new InvalidOperationException("Temporary file has not been created");
204201

205-
Logger.DebugFormat("FilePartDataHandler: Opening file for writing at offset {0} with BufferSize={1}",
202+
_logger.DebugFormat("FilePartDataHandler: Opening file for writing at offset {0} with BufferSize={1}",
206203
offset, _config.BufferSize);
207204

208205
// Open file with FileShare.Write to allow concurrent writes from other threads
@@ -216,7 +213,7 @@ private async Task WritePartToFileAsync(
216213
// Seek to the correct offset for this part
217214
fileStream.Seek(offset, SeekOrigin.Begin);
218215

219-
Logger.DebugFormat("FilePartDataHandler: Writing {0} bytes to file at offset {1}",
216+
_logger.DebugFormat("FilePartDataHandler: Writing {0} bytes to file at offset {1}",
220217
response.ContentLength, offset);
221218

222219
// Use GetObjectResponse's stream copy logic which includes:
@@ -235,7 +232,7 @@ await response.WriteResponseStreamAsync(
235232
await fileStream.FlushAsync(cancellationToken)
236233
.ConfigureAwait(false);
237234

238-
Logger.DebugFormat("FilePartDataHandler: Successfully wrote {0} bytes at offset {1}",
235+
_logger.DebugFormat("FilePartDataHandler: Successfully wrote {0} bytes at offset {1}",
239236
response.ContentLength, offset);
240237
}
241238
}

sdk/src/Services/S3/Custom/Transfer/Internal/MultipartDownloadCommand.cs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,7 @@ internal partial class MultipartDownloadCommand : BaseCommand<TransferUtilityDow
4242
// Track last known transferred bytes from coordinator's progress events
4343
private long _lastKnownTransferredBytes;
4444

45-
private static Logger Logger
46-
{
47-
get
48-
{
49-
return Logger.GetLogger(typeof(TransferUtility));
50-
}
51-
}
45+
private readonly Logger _logger = Logger.GetLogger(typeof(MultipartDownloadCommand));
5246

5347
/// <summary>
5448
/// Initializes a new instance of the MultipartDownloadCommand class for single file downloads.
@@ -118,7 +112,7 @@ private FileDownloadConfiguration CreateConfiguration()
118112
// Use S3 client buffer size for I/O operations
119113
int bufferSize = _s3Client.Config.BufferSize;
120114

121-
Logger.DebugFormat("MultipartDownloadCommand: Creating configuration - PartSizeFromRequest={0}, UsingDefaultPartSize={1}",
115+
_logger.DebugFormat("MultipartDownloadCommand: Creating configuration - PartSizeFromRequest={0}, UsingDefaultPartSize={1}",
122116
_request.IsSetPartSize() ? _request.PartSize.ToString() : "Not Set",
123117
!_request.IsSetPartSize());
124118

0 commit comments

Comments
 (0)