Skip to content

Commit

Permalink
✨ Add logging in the GRPC image service.
Browse files Browse the repository at this point in the history
  • Loading branch information
hexawyz committed Jan 19, 2025
1 parent edb34ef commit 96e7b11
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 29 deletions.
23 changes: 19 additions & 4 deletions src/Exo/Service/Exo.Service.Core/ImageStorageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading.Channels;
using Exo.Configuration;
using Exo.Images;
using Microsoft.Extensions.Logging;

namespace Exo.Service;

Expand All @@ -23,7 +24,13 @@ private readonly struct ImageMetadata(UInt128 id, ushort width, ushort height, I

private static string GetFileName(string imageCacheDirectory, UInt128 imageId) => Path.Combine(imageCacheDirectory, imageId.ToString("x32", CultureInfo.InvariantCulture));

public static async Task<ImageStorageService> CreateAsync(IConfigurationContainer<string> imagesConfigurationContainer, string imageCacheDirectory, CancellationToken cancellationToken)
public static async Task<ImageStorageService> CreateAsync
(
ILogger<ImageStorageService> logger,
IConfigurationContainer<string> imagesConfigurationContainer,
string imageCacheDirectory,
CancellationToken cancellationToken
)
{
if (!Path.IsPathRooted(imageCacheDirectory)) throw new ArgumentException("Images directory path must be rooted.");

Expand All @@ -49,17 +56,25 @@ public static async Task<ImageStorageService> CreateAsync(IConfigurationContaine
}
}

return new(imagesConfigurationContainer, imageCacheDirectory, imageCollection);
return new(logger, imagesConfigurationContainer, imageCacheDirectory, imageCollection);
}

private readonly Dictionary<string, ImageMetadata> _imageCollection;
private readonly IConfigurationContainer<string> _imagesConfigurationContainer;
private readonly string _imageCacheDirectory;
private ChannelWriter<ImageChangeNotification>[]? _changeListeners;
private readonly AsyncLock _lock;

private ImageStorageService(IConfigurationContainer<string> imagesConfigurationContainer, string imageCacheDirectory, Dictionary<string, ImageMetadata> imageCollection)
private readonly ILogger<ImageStorageService> _logger;

private ImageStorageService
(
ILogger<ImageStorageService> logger,
IConfigurationContainer<string> imagesConfigurationContainer,
string imageCacheDirectory,
Dictionary<string, ImageMetadata> imageCollection
)
{
_logger = logger;
_imagesConfigurationContainer = imagesConfigurationContainer;
_imageCacheDirectory = imageCacheDirectory;
_imageCollection = imageCollection;
Expand Down
86 changes: 61 additions & 25 deletions src/Exo/Service/Exo.Service.Grpc/GrpcImageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,100 @@
using System.Runtime.CompilerServices;
using Exo.Contracts.Ui.Settings;
using Exo.Memory;
using Microsoft.Extensions.Logging;

namespace Exo.Service.Grpc;

internal sealed class GrpcImageService : IImageService
{
private readonly ImageStorageService _imageStorageService;
private readonly ILogger<GrpcImageService> _logger;
private ImageAddState? _imageAddState;

public GrpcImageService(ImageStorageService imagesService) => _imageStorageService = imagesService;
public GrpcImageService(ILogger<GrpcImageService> logger, ImageStorageService imagesService)
{
_logger = logger;
_imageStorageService = imagesService;
}

public async IAsyncEnumerable<WatchNotification<Contracts.Ui.Settings.ImageInformation>> WatchImagesAsync([EnumeratorCancellation] CancellationToken cancellationToken)
{
await foreach (var notification in _imageStorageService.WatchChangesAsync(cancellationToken).ConfigureAwait(false))
_logger.GrpcImageServiceImageWatchStart();
try
{
yield return new()
await foreach (var notification in _imageStorageService.WatchChangesAsync(cancellationToken).ConfigureAwait(false))
{
NotificationKind = notification.Kind.ToGrpc(),
Details = notification.ImageInformation.ToGrpc(),
};
yield return new()
{
NotificationKind = notification.Kind.ToGrpc(),
Details = notification.ImageInformation.ToGrpc(),
};
}
}
finally
{
_logger.GrpcImageServiceImageWatchStop();
}
}

public async IAsyncEnumerable<ImageRegistrationBeginResponse> BeginAddImageAsync(ImageRegistrationBeginRequest request, [EnumeratorCancellation] CancellationToken cancellationToken)
{
if (await _imageStorageService.HasImageAsync(request.ImageName, cancellationToken).ConfigureAwait(false))
_logger.GrpcImageServiceAddImageRequestStart(request.ImageName, request.Length);

ImageAddState state;

try
{
throw new InvalidOperationException("An image with the specified name is already present.");
}
// 24MB is the maximum image size that could be theoretically allocated on the NZXT Kraken Z, so it seems reasonable to fix this as the limit for now?
// This number is semi-arbitrary, as we have to allow somewhat large images, but we don't want to allow anything crazy.
// The risk is someone abusing the service to waste memory. Stupid but possible.
// We have to be careful because apps cannot create shared memory in the Global namespace themselves, so the service needs to handle it ☹️
// This is also the reason why only a single parallel request is allowed. Anyway, this would never be a problem in practice as the UI is supposed to be the unique client.
if (request.Length > 24 * 1024 * 1024) throw new Exception("Maximum allowed image size exceeded.");
if (await _imageStorageService.HasImageAsync(request.ImageName, cancellationToken).ConfigureAwait(false))
{
throw new InvalidOperationException("An image with the specified name is already present.");
}
// 24MB is the maximum image size that could be theoretically allocated on the NZXT Kraken Z, so it seems reasonable to fix this as the limit for now?
// This number is semi-arbitrary, as we have to allow somewhat large images, but we don't want to allow anything crazy.
// The risk is someone abusing the service to waste memory. Stupid but possible.
// We have to be careful because apps cannot create shared memory in the Global namespace themselves, so the service needs to handle it ☹️
// This is also the reason why only a single parallel request is allowed. Anyway, this would never be a problem in practice as the UI is supposed to be the unique client.
if (request.Length > 24 * 1024 * 1024) throw new Exception("Maximum allowed image size exceeded.");

var state = new ImageAddState(request.ImageName);
state = new ImageAddState(request.ImageName);

if (Interlocked.CompareExchange(ref _imageAddState, state, null) is not null)
if (Interlocked.CompareExchange(ref _imageAddState, state, null) is not null)
{
state.Dispose();
throw new InvalidOperationException("Another operation is currently pending.");
}
}
catch (Exception ex)
{
state.Dispose();
throw new InvalidOperationException("Another operation is currently pending.");
_logger.GrpcImageServiceAddImageRequestFailure(request.ImageName, ex);
throw;
}

try
{
yield return new() { RequestId = state.RequestId, SharedMemoryName = state.Initialize(request.Length) };
await state.WaitForWriteCompletion().WaitAsync(cancellationToken).ConfigureAwait(false);
try
{
using (var memoryManager = state.CreateMemoryManagerForRead())
await state.WaitForWriteCompletion().WaitAsync(cancellationToken).ConfigureAwait(false);
_logger.GrpcImageServiceAddImageRequestWriteComplete(request.ImageName);
try
{
using (var memoryManager = state.CreateMemoryManagerForRead())
{
await _imageStorageService.AddImageAsync(request.ImageName, memoryManager.Memory, cancellationToken).ConfigureAwait(false);
}
_logger.GrpcImageServiceAddImageRequestSuccess(request.ImageName);
state.TryNotifyReadCompletion();
}
catch (Exception ex)
{
await _imageStorageService.AddImageAsync(request.ImageName, memoryManager.Memory, cancellationToken).ConfigureAwait(false);
state.TryNotifyReadException(ex);
}
state.TryNotifyReadCompletion();
}
catch (Exception ex)
catch (Exception ex) when (ex is not OperationCanceledException)
{
state.TryNotifyReadException(ex);
_logger.GrpcImageServiceAddImageRequestFailure(request.ImageName, ex);
throw;
}
}
finally
Expand Down
18 changes: 18 additions & 0 deletions src/Exo/Service/Exo.Service.Grpc/LoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,22 @@ internal static partial class LoggerExtensions
[LoggerMessage(EventId = 10_704, EventName = "GrpcCoolingServiceChangeWatchStop", Level = LogLevel.Debug, Message = "Stopped watching cooling changes.")]
public static partial void GrpcCoolingServiceChangeWatchStop(this ILogger logger);

[LoggerMessage(EventId = 10_801, EventName = "GrpcImageServiceImageWatchStart", Level = LogLevel.Debug, Message = "Started watching image changes.")]
public static partial void GrpcImageServiceImageWatchStart(this ILogger logger);

[LoggerMessage(EventId = 10_802, EventName = "GrpcImageServiceImageWatchStop", Level = LogLevel.Debug, Message = "Stopped watching image changes.")]
public static partial void GrpcImageServiceImageWatchStop(this ILogger logger);

[LoggerMessage(EventId = 10_803, EventName = "GrpcImageServiceAddImageRequestStart", Level = LogLevel.Information, Message = "Start of request to add image {ImageName} of {imageDataLength} bytes.")]
public static partial void GrpcImageServiceAddImageRequestStart(this ILogger logger, string imageName, uint imageDataLength);

[LoggerMessage(EventId = 10_804, EventName = "GrpcImageServiceAddImageRequestWriteComplete", Level = LogLevel.Debug, Message = "Notification of image data written for request to add image {ImageName}.")]
public static partial void GrpcImageServiceAddImageRequestWriteComplete(this ILogger logger, string imageName);

[LoggerMessage(EventId = 10_805, EventName = "GrpcImageServiceAddImageRequestFailure", Level = LogLevel.Error, Message = "Failure to process request to add image {ImageName}.")]
public static partial void GrpcImageServiceAddImageRequestFailure(this ILogger logger, string imageName, Exception exception);

[LoggerMessage(EventId = 10_806, EventName = "GrpcImageServiceAddImageRequestSuccess", Level = LogLevel.Information, Message = "Success of request to add image {ImageName}.")]
public static partial void GrpcImageServiceAddImageRequestSuccess(this ILogger logger, string imageName);

}
1 change: 1 addition & 0 deletions src/Exo/Service/Exo.Service/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ public void ConfigureServices(IServiceCollection services)
(
sp => ImageStorageService.CreateAsync
(
sp.GetRequiredService<ILogger<ImageStorageService>>(),
sp.GetRequiredKeyedService<IConfigurationContainer<string>>(ConfigurationContainerNames.Images),
Path.Combine(baseDirectory, "img"),
default
Expand Down

0 comments on commit 96e7b11

Please sign in to comment.