Skip to content

Fix Logger Recursive property definition #4188

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.

Sign up for GitHub

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

Merged
GarrettBeatty merged 1 commit into from
Dec 3, 2025
Merged

Fix Logger Recursive property definition #4188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 generator/.DevConfigs/c49077d9-90b3-437f-b316-6d8d8833ae65.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"services": [
{
"serviceName": "S3",
"type": "patch",
"changeLogMessages": [
"Fix Transfer Utility internal Logger recursive property definition"
]
}
]
}
17 changes: 7 additions & 10 deletions sdk/src/Services/S3/Custom/Transfer/Internal/BufferedMultipartStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ internal class BufferedMultipartStream : Stream
private DownloadDiscoveryResult _discoveryResult;
private long _totalBytesRead = 0;

private Logger Logger
{
get { return Logger.GetLogger(typeof(TransferUtility)); }
}
private readonly Logger _logger = Logger.GetLogger(typeof(BufferedMultipartStream));

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

Logger.DebugFormat("BufferedMultipartStream: Starting initialization");
_logger.DebugFormat("BufferedMultipartStream: Starting initialization");

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

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

_initialized = true;
Logger.DebugFormat("BufferedMultipartStream: Initialization completed successfully");
_logger.DebugFormat("BufferedMultipartStream: Initialization completed successfully");
}

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

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

var bytesRead = await _partBufferManager.ReadAsync(buffer, offset, count, cancellationToken)
Expand All @@ -178,12 +175,12 @@ public override async Task<int> ReadAsync(byte[] buffer, int offset, int count,
if (bytesRead > 0)
{
Interlocked.Add(ref _totalBytesRead, bytesRead);
Logger.DebugFormat("BufferedMultipartStream: ReadAsync completed - BytesRead={0}, NewPosition={1}",
_logger.DebugFormat("BufferedMultipartStream: ReadAsync completed - BytesRead={0}, NewPosition={1}",
bytesRead, currentPosition + bytesRead);
}
else
{
Logger.DebugFormat("BufferedMultipartStream: ReadAsync returned EOF (0 bytes)");
_logger.DebugFormat("BufferedMultipartStream: ReadAsync returned EOF (0 bytes)");
}

return bytesRead;
Expand Down
29 changes: 13 additions & 16 deletions sdk/src/Services/S3/Custom/Transfer/Internal/BufferedPartDataHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ internal class BufferedPartDataHandler : IPartDataHandler
private readonly IPartBufferManager _partBufferManager;
private readonly BufferedDownloadConfiguration _config;

private Logger Logger
{
get { return Logger.GetLogger(typeof(TransferUtility)); }
}
private readonly Logger _logger = Logger.GetLogger(typeof(BufferedPartDataHandler));

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

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

Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] StreamingDataSource added and capacity released",
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] StreamingDataSource added and capacity released",
partNumber);
}
catch (Exception ex)
{
Logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process streaming part", partNumber);
_logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process streaming part", partNumber);

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

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

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

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

Logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Added to buffer manager (capacity will be released after consumption)",
_logger.DebugFormat("BufferedPartDataHandler: [Part {0}] Added to buffer manager (capacity will be released after consumption)",
partNumber);
}
catch (Exception ex)
{
Logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process buffered part", partNumber);
_logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to process buffered part", partNumber);

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

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

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

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

int totalRead = (int)memoryStream.Position;

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

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

if (totalRead != expectedBytes)
{
Logger.Error(null, "BufferedPartDataHandler: [Part {0}] Size mismatch - Expected {1} bytes, read {2} bytes",
_logger.Error(null, "BufferedPartDataHandler: [Part {0}] Size mismatch - Expected {1} bytes, read {2} bytes",
partNumber, expectedBytes, totalRead);
}
}
Expand All @@ -333,7 +330,7 @@ await response.WriteResponseStreamAsync(
}
catch (Exception ex)
{
Logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to buffer part from response stream", partNumber);
_logger.Error(ex, "BufferedPartDataHandler: [Part {0}] Failed to buffer part from response stream", partNumber);
// If something goes wrong, StreamPartBuffer.Dispose() will handle cleanup
downloadedPart?.Dispose();
throw;
Expand Down
27 changes: 12 additions & 15 deletions sdk/src/Services/S3/Custom/Transfer/Internal/FilePartDataHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ internal class FilePartDataHandler : IPartDataHandler
private string _tempFilePath;
private bool _disposed = false;

private Logger Logger
{
get { return Logger.GetLogger(typeof(TransferUtility)); }
}
private readonly Logger _logger = Logger.GetLogger(typeof(FilePartDataHandler));

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

Logger.DebugFormat("FilePartDataHandler: Created temporary file for download");
_logger.DebugFormat("FilePartDataHandler: Created temporary file for download");

return Task.CompletedTask;
}
Expand All @@ -83,20 +80,20 @@ public async Task ProcessPartAsync(
{
try
{
Logger.DebugFormat("FilePartDataHandler: [Part {0}] Starting to process part - ContentLength={1}",
_logger.DebugFormat("FilePartDataHandler: [Part {0}] Starting to process part - ContentLength={1}",
partNumber, response.ContentLength);

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

Logger.DebugFormat("FilePartDataHandler: [Part {0}] Calculated file offset={1}",
_logger.DebugFormat("FilePartDataHandler: [Part {0}] Calculated file offset={1}",
partNumber, offset);

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

Logger.DebugFormat("FilePartDataHandler: [Part {0}] File write completed successfully",
_logger.DebugFormat("FilePartDataHandler: [Part {0}] File write completed successfully",
partNumber);
}
finally
Expand Down Expand Up @@ -128,17 +125,17 @@ public void OnDownloadComplete(Exception exception)
if (exception == null)
{
// Success - commit temp file to final destination
Logger.DebugFormat("FilePartDataHandler: Download complete, committing temporary file to destination");
_logger.DebugFormat("FilePartDataHandler: Download complete, committing temporary file to destination");

try
{
_fileHandler.CommitFile(_tempFilePath, _config.DestinationFilePath);

Logger.DebugFormat("FilePartDataHandler: Successfully committed file to destination");
_logger.DebugFormat("FilePartDataHandler: Successfully committed file to destination");
}
catch (Exception commitException)
{
Logger.Error(commitException, "FilePartDataHandler: Failed to commit file to destination");
_logger.Error(commitException, "FilePartDataHandler: Failed to commit file to destination");

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

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

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

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

Logger.DebugFormat("FilePartDataHandler: Writing {0} bytes to file at offset {1}",
_logger.DebugFormat("FilePartDataHandler: Writing {0} bytes to file at offset {1}",
response.ContentLength, offset);

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

Logger.DebugFormat("FilePartDataHandler: Successfully wrote {0} bytes at offset {1}",
_logger.DebugFormat("FilePartDataHandler: Successfully wrote {0} bytes at offset {1}",
response.ContentLength, offset);
}
}
Expand Down
10 changes: 2 additions & 8 deletions sdk/src/Services/S3/Custom/Transfer/Internal/MultipartDownloadCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,7 @@ internal partial class MultipartDownloadCommand : BaseCommand<TransferUtilityDow
// Track last known transferred bytes from coordinator's progress events
private long _lastKnownTransferredBytes;

private static Logger Logger
{
get
{
return Logger.GetLogger(typeof(TransferUtility));
}
}
private readonly Logger _logger = Logger.GetLogger(typeof(MultipartDownloadCommand));

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

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

Expand Down
Loading