Merge pull request #15 from bdach/observability

Improve observability by integrating sentry, datadog, and adding extended logging

README.md

@@ -39,6 +39,8 @@ For advanced testing purposes.
 | `SHARED_INTEROP_SECRET`      | The interop secret used for legacy IO requests. Value should match same environment variable in target `osu-web` instance.                         |              ✔️ Yes              | None          |
 | `S3_ACCESS_KEY`              | A valid Amazon S3 access key ID.                                                                                                                   | ⚠ In staging/production configs  | None          |
 | `S3_SECRET_KEY`              | The secret key corresponding to the `S3_ACCESS_KEY`.                                                                                               | ⚠ In staging/production configs  | None          |
-| `S3_CENTRAL_BUCKET_NAME`     | The name of the S3 bucket to use for storing beatmap packages and versioned files.                                                                 | ⚠ In staging/production configs  | None          | 
-| `S3_BEATMAPS_BUCKET_NAME`    | The name of the S3 bucket to use for storing .osu beatmap files.                                                                                   | ⚠ In staging/production configs  | None          | 
+| `S3_CENTRAL_BUCKET_NAME`     | The name of the S3 bucket to use for storing beatmap packages and versioned files.                                                                 | ⚠ In staging/production configs  | None          |
+| `S3_BEATMAPS_BUCKET_NAME`    | The name of the S3 bucket to use for storing .osu beatmap files.                                                                                   | ⚠ In staging/production configs  | None          |
+| `SENTRY_DSN`                 | A valid Sentry DSN to use for logging application events.                                                                                          | ⚠ In staging/production configs  | None          | 
+| `DD_AGENT_HOST`              | A hostname pointing to a Datadog agent instance to which metrics should be reported.                                                               | ⚠ In staging/production configs  | None          | 
 

osu.Server.BeatmapSubmission/Configuration/AppSettings.cs

@@ -45,5 +45,9 @@ public static class AppSettings
             Environment.GetEnvironmentVariable("S3_BEATMAPS_BUCKET_NAME")
             ?? throw new InvalidOperationException("S3_BEATMAPS_BUCKET_NAME environment variable not set. "
                                                    + "Please set the value of this variable to the name of the bucket to be used for storing .osu beatmap files on S3.");
+
+        public static string? SentryDsn => Environment.GetEnvironmentVariable("SENTRY_DSN");
+
+        public static string? DatadogAgentHost => Environment.GetEnvironmentVariable("DD_AGENT_HOST");
     }
 }

osu.Server.BeatmapSubmission/DatabaseOperationExtensions.cs

@@ -65,7 +65,7 @@ public static async Task<ulong> GetUserMonthlyPlaycountAsync(this MySqlConnectio
                 transaction) ?? 0;
         }
 
-        public static async Task PurgeInactiveBeatmapSetsForUserAsync(this MySqlConnection db, uint userId, MySqlTransaction? transaction = null)
+        public static async Task<uint[]> PurgeInactiveBeatmapSetsForUserAsync(this MySqlConnection db, uint userId, MySqlTransaction? transaction = null)
         {
             uint[] beatmapSetIds = (await db.QueryAsync<uint>(@"SELECT `beatmapset_id` FROM `osu_beatmapsets` WHERE `user_id` = @user_id AND `active` = -1 AND `deleted_at` IS NULL",
                 new
@@ -86,6 +86,8 @@ await db.ExecuteAsync(@"DELETE FROM `osu_beatmapsets` WHERE `user_id` = @user_id
                     user_id = userId,
                 },
                 transaction);
+
+            return beatmapSetIds;
         }
 
         public static Task<(uint unranked, uint ranked)> GetUserBeatmapSetCountAsync(this MySqlConnection db, uint userId, MySqlTransaction? transaction = null)

osu.Server.BeatmapSubmission/InvariantExceptionFilter.cs

@@ -8,10 +8,20 @@ namespace osu.Server.BeatmapSubmission
 {
     public class InvariantExceptionFilter : IExceptionFilter
     {
+        private readonly ILogger<InvariantExceptionFilter> logger;
+
+        public InvariantExceptionFilter(ILogger<InvariantExceptionFilter> logger)
+        {
+            this.logger = logger;
+        }
+
         public void OnException(ExceptionContext context)
         {
             if (context.Exception is InvariantException invariantException)
+            {
                 context.Result = invariantException.ToResponseObject().ToActionResult();
+                logger.Log(invariantException.Severity, context.Exception, "Request rejected due to violated invariant");
+            }
         }
     }
 }

osu.Server.BeatmapSubmission/Logging/UserFactory.cs

@@ -0,0 +1,32 @@
+// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Globalization;
+using System.Security.Claims;
+using osu.Server.BeatmapSubmission.Authentication;
+
+namespace osu.Server.BeatmapSubmission.Logging
+{
+    public class UserFactory : ISentryUserFactory
+    {
+        private readonly IHttpContextAccessor httpContextAccessor;
+
+        public UserFactory(IHttpContextAccessor httpContextAccessor)
+        {
+            this.httpContextAccessor = httpContextAccessor;
+        }
+
+        public SentryUser? Create()
+        {
+            var user = httpContextAccessor.HttpContext?.User;
+
+            if (user == null || !user.HasClaim(claim => claim.Type == ClaimTypes.NameIdentifier))
+                return null;
+
+            return new SentryUser
+            {
+                Id = user.GetUserId().ToString(CultureInfo.InvariantCulture),
+            };
+        }
+    }
+}

osu.Server.BeatmapSubmission/Models/InvariantException.cs

@@ -11,9 +11,12 @@ namespace osu.Server.BeatmapSubmission.Models
     /// </summary>
     public class InvariantException : Exception
     {
-        public InvariantException(string message)
+        public LogLevel Severity { get; }
+
+        public InvariantException(string message, LogLevel severity = LogLevel.Information)
             : base(message)
         {
+            Severity = severity;
         }
 
         public ErrorResponse ToResponseObject() => new ErrorResponse(Message);

osu.Server.BeatmapSubmission/Program.cs

@@ -1,9 +1,13 @@
+using System.Net;
 using System.Reflection;
 using JetBrains.Annotations;
 using Microsoft.AspNetCore.Authentication.JwtBearer;
 using Microsoft.Extensions.Options;
 using osu.Server.BeatmapSubmission.Authentication;
+using osu.Server.BeatmapSubmission.Configuration;
+using osu.Server.BeatmapSubmission.Logging;
 using osu.Server.BeatmapSubmission.Services;
+using StatsdClient;
 
 namespace osu.Server.BeatmapSubmission
 {
@@ -24,7 +28,8 @@ public static void Main(string[] args)
             {
                 logging.ClearProviders();
                 logging.AddConsole();
-                // TODO: sentry
+                if (AppSettings.SentryDsn != null)
+                    logging.AddSentry();
             });
 
             builder.Services.AddSingleton<IConfigureOptions<JwtBearerOptions>, OsuWebSharedJwtBearerOptions>();
@@ -59,10 +64,48 @@ public static void Main(string[] args)
                     builder.Services.AddHttpClient();
                     builder.Services.AddTransient<ILegacyIO, LegacyIO>();
                     builder.Services.AddTransient<IMirrorService, MirrorService>();
+
+                    if (AppSettings.SentryDsn == null)
+                    {
+                        throw new InvalidOperationException("SENTRY_DSN environment variable not set. "
+                                                            + "Please set the value of this variable to a valid Sentry DSN to use for logging events.");
+                    }
+
+                    if (AppSettings.DatadogAgentHost == null)
+                    {
+                        throw new InvalidOperationException("DD_AGENT_HOST environment variable not set. "
+                                                            + "Please set the value of this variable to a valid hostname of a Datadog agent.");
+                    }
+
                     break;
                 }
             }
 
+            if (AppSettings.SentryDsn != null)
+            {
+                builder.Services.AddSingleton<ISentryUserFactory, UserFactory>();
+                builder.WebHost.UseSentry(options =>
+                {
+                    options.Environment = builder.Environment.EnvironmentName;
+                    options.SendDefaultPii = true;
+                    options.Dsn = AppSettings.SentryDsn;
+                });
+            }
+
+            if (AppSettings.DatadogAgentHost != null)
+            {
+                DogStatsd.Configure(new StatsdConfig
+                {
+                    StatsdServerName = AppSettings.DatadogAgentHost,
+                    Prefix = "osu.server.beatmap-submission",
+                    ConstantTags = new[]
+                    {
+                        $@"hostname:{Dns.GetHostName()}",
+                        $@"startup:{DateTimeOffset.UtcNow.ToUnixTimeSeconds()}"
+                    }
+                });
+            }
+
             var app = builder.Build();
 
             if (app.Environment.IsDevelopment())

osu.Server.BeatmapSubmission/Services/BeatmapPackagePatcher.cs

@@ -83,7 +83,7 @@ public async Task<MemoryStream> PatchBeatmapAsync(
             await beatmapStorage.ExtractBeatmapSetAsync(beatmapSetId, tempDirectory.FullName);
 
             if (beatmap.filename == null)
-                throw new InvalidOperationException("Could not find the old .osu file for the beatmap being modified.");
+                throw new InvariantException("Could not find the old .osu file for the beatmap being modified.", LogLevel.Warning);
 
             File.Delete(Path.Combine(tempDirectory.FullName, beatmap.filename));
 

osu.Server.BeatmapSubmission/Services/LegacyIO.cs

@@ -12,10 +12,12 @@ namespace osu.Server.BeatmapSubmission.Services
     public class LegacyIO : ILegacyIO
     {
         private readonly HttpClient httpClient;
+        private readonly ILogger<LegacyIO> logger;
 
-        public LegacyIO(HttpClient httpClient)
+        public LegacyIO(HttpClient httpClient, ILogger<LegacyIO> logger)
         {
             this.httpClient = httpClient;
+            this.logger = logger;
         }
 
         private async Task runLegacyIO(HttpMethod method, string command, dynamic? postObject = null)
@@ -27,6 +29,9 @@ private async Task runLegacyIO(HttpMethod method, string command, dynamic? postO
             long time = DateTimeOffset.UtcNow.ToUnixTimeSeconds();
             string url = $"{AppSettings.LegacyIODomain}/_lio/{command}{(command.Contains('?') ? "&" : "?")}timestamp={time}";
 
+            string? serialisedPostObject = postObject == null ? null : JsonSerializer.Serialize(postObject);
+            logger.LogDebug("Performing LIO request to {method} {url} (params: {params})", method, url, serialisedPostObject);
+
             try
             {
                 string signature = hmacEncode(url, Encoding.UTF8.GetBytes(AppSettings.SharedInteropSecret));
@@ -42,9 +47,9 @@ private async Task runLegacyIO(HttpMethod method, string command, dynamic? postO
                     },
                 };
 
-                if (postObject != null)
+                if (serialisedPostObject != null)
                 {
-                    httpRequestMessage.Content = new ByteArrayContent(Encoding.UTF8.GetBytes(JsonSerializer.Serialize(postObject)));
+                    httpRequestMessage.Content = new ByteArrayContent(Encoding.UTF8.GetBytes(serialisedPostObject));
                     httpRequestMessage.Content.Headers.ContentType = MediaTypeHeaderValue.Parse("application/json");
                 }
 
@@ -60,7 +65,7 @@ private async Task runLegacyIO(HttpMethod method, string command, dynamic? postO
             {
                 if (retryCount-- > 0)
                 {
-                    Console.WriteLine($"Legacy IO request to {url} failed with {e}, retrying..");
+                    logger.LogError(e, "Legacy IO request to {url} failed, retrying ({retries} remaining)", url, retryCount);
                     Thread.Sleep(1000);
                     goto retry;
                 }

osu.Server.BeatmapSubmission/Services/MirrorService.cs

@@ -10,10 +10,12 @@ namespace osu.Server.BeatmapSubmission.Services
     public class MirrorService : IMirrorService
     {
         private readonly HttpClient client;
+        private readonly ILogger<MirrorService> logger;
 
-        public MirrorService(HttpClient client)
+        public MirrorService(HttpClient client, ILogger<MirrorService> logger)
         {
             this.client = client;
+            this.logger = logger;
         }
 
         public async Task PurgeBeatmapSetAsync(MySqlConnection db, uint beatmapSetId)
@@ -29,6 +31,8 @@ public async Task PurgeBeatmapSetAsync(MySqlConnection db, uint beatmapSetId)
 
         private async Task<string?> performMirrorAction(osu_mirror mirror, string action, Dictionary<string, string> data)
         {
+            logger.LogInformation("Performing action {action} (data: {data}) on mirror {mirror}", action, data, mirror.base_url);
+
             data["ts"] = DateTimeOffset.Now.ToUnixTimeSeconds().ToString();
             data["action"] = action;
             data["cs"] = ($"{data.GetValueOrDefault("s")}{data.GetValueOrDefault("fd")}{data.GetValueOrDefault("fs")}{data.GetValueOrDefault("ts")}"
@@ -42,9 +46,9 @@ public async Task PurgeBeatmapSetAsync(MySqlConnection db, uint beatmapSetId)
                 var response = await client.SendAsync(request);
                 return await response.Content.ReadAsStringAsync();
             }
-            catch (Exception)
+            catch (Exception e)
             {
-                // TODO: log error
+                logger.LogError(e, "Attempting to perform action {action} on mirror {mirror} failed", action, mirror.base_url);
                 return null;
             }
         }

osu.Server.BeatmapSubmission/Services/S3BeatmapStorage.cs

@@ -20,10 +20,12 @@ public class S3BeatmapStorage : IBeatmapStorage
         private const string osz_directory = "osz";
         private const string versioned_file_directory = "beatmap_files";
 
+        private readonly ILogger<S3BeatmapStorage> logger;
         private readonly AmazonS3Client client;
 
-        public S3BeatmapStorage()
+        public S3BeatmapStorage(ILogger<S3BeatmapStorage> logger)
         {
+            this.logger = logger;
             client = new AmazonS3Client(
                 new BasicAWSCredentials(AppSettings.S3AccessKey, AppSettings.S3SecretKey),
                 new AmazonS3Config
@@ -58,15 +60,17 @@ public async Task StoreBeatmapSetAsync(uint beatmapSetId, byte[] beatmapPackage,
                     beatmapFiles.Add((beatmapId, contents));
             }
 
-            await Task.WhenAll([
-                uploadBeatmapPackage(beatmapSetId, beatmapPackage, stream),
-                ..uploadAllVersionedFiles(allFiles),
-                ..uploadAllBeatmapFiles(beatmapFiles)
-            ]);
+            var tasks = new List<Task> { uploadBeatmapPackage(beatmapSetId, beatmapPackage, stream) };
+            tasks.AddRange(uploadAllVersionedFiles(beatmapSetId, allFiles));
+            tasks.AddRange(uploadAllBeatmapFiles(beatmapSetId, beatmapFiles));
+            await Task.WhenAll(tasks);
+
+            logger.LogInformation("All file uploads for beatmapset {beatmapSetId} concluded successfully.", beatmapSetId);
         }
 
         private Task<PutObjectResponse> uploadBeatmapPackage(uint beatmapSetId, byte[] beatmapPackage, MemoryStream stream)
         {
+            logger.LogInformation("Beginning upload of package for beatmapset {beatmapSetId}...", beatmapSetId);
             return client.PutObjectAsync(new PutObjectRequest
             {
                 BucketName = AppSettings.S3CentralBucketName,
@@ -81,8 +85,9 @@ private Task<PutObjectResponse> uploadBeatmapPackage(uint beatmapSetId, byte[] b
             });
         }
 
-        private IEnumerable<Task> uploadAllVersionedFiles(List<byte[]> files)
+        private IEnumerable<Task> uploadAllVersionedFiles(uint beatmapSetId, List<byte[]> files)
         {
+            logger.LogInformation("Beginning upload of all versioned files for beatmapset {beatmapSetId}...", beatmapSetId);
             return files.Select(file => Task.Run(async () =>
             {
                 var fileStream = new MemoryStream(file);
@@ -103,8 +108,9 @@ await client.PutObjectAsync(new PutObjectRequest
             }));
         }
 
-        private IEnumerable<Task> uploadAllBeatmapFiles(List<(int beatmapId, byte[] contents)> beatmapFiles)
+        private IEnumerable<Task> uploadAllBeatmapFiles(uint beatmapSetId, List<(int beatmapId, byte[] contents)> beatmapFiles)
         {
+            logger.LogInformation("Beginning upload of all .osu beatmap files for beatmapset {beatmapSetId}...", beatmapSetId);
             return beatmapFiles.Select(file => Task.Run(async () =>
             {
                 var fileStream = new MemoryStream(file.contents);
@@ -127,7 +133,9 @@ await client.PutObjectAsync(new PutObjectRequest
 
         public async Task ExtractBeatmapSetAsync(uint beatmapSetId, string targetDirectory)
         {
+            logger.LogInformation("Retrieving package for beatmap set {beatmapSetId}", beatmapSetId);
             using var response = await client.GetObjectAsync(AppSettings.S3CentralBucketName, getPathToPackage(beatmapSetId));
+            logger.LogInformation("Package for beatmap set {beatmapSetId} retrieved successfully.", beatmapSetId);
             // S3-provided `HashStream` does not support seeking which `ZipArchiveReader` does not like.
             var memoryStream = new MemoryStream(await response.ResponseStream.ReadAllRemainingBytesToArrayAsync());
 
@@ -151,15 +159,20 @@ public async Task<Stream> PackageBeatmapSetFilesAsync(IEnumerable<PackageFile> f
         {
             var memoryStream = new MemoryStream();
 
+            logger.LogInformation("Retrieving requested files...");
+
             using (var zipWriter = new ZipWriter(memoryStream, BeatmapPackagePatcher.DEFAULT_ZIP_WRITER_OPTIONS))
             {
                 foreach (var file in files)
                 {
-                    using var response = await client.GetObjectAsync(AppSettings.S3CentralBucketName, getPathToVersionedFile(BitConverter.ToString(file.File.sha2_hash).Replace("-", string.Empty).ToLowerInvariant()));
+                    using var response = await client.GetObjectAsync(AppSettings.S3CentralBucketName,
+                        getPathToVersionedFile(BitConverter.ToString(file.File.sha2_hash).Replace("-", string.Empty).ToLowerInvariant()));
                     zipWriter.Write(file.VersionFile.filename, response.ResponseStream);
                 }
             }
 
+            logger.LogInformation("Package files retrieved successfully.");
+
             memoryStream.Seek(0, SeekOrigin.Begin);
             return memoryStream;
         }

osu.Server.BeatmapSubmission/osu.Server.BeatmapSubmission.csproj

@@ -16,6 +16,7 @@
         <PackageReference Include="BouncyCastle.Cryptography" Version="2.5.0" />
         <PackageReference Include="Dapper" Version="2.1.35" />
         <PackageReference Include="Dapper.Contrib" Version="2.0.78" />
+        <PackageReference Include="DogStatsD-CSharp-Client" Version="8.0.0" />
         <PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="8.0.10" />
         <PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="8.0.10"/>
         <PackageReference Include="ppy.osu.Game" Version="2024.1130.0" />
@@ -24,6 +25,7 @@
         <PackageReference Include="ppy.osu.Game.Rulesets.Osu" Version="2024.1130.0" />
         <PackageReference Include="ppy.osu.Game.Rulesets.Taiko" Version="2024.1130.0" />
         <PackageReference Include="ppy.osu.Server.OsuQueueProcessor" Version="2024.1111.0" />
+        <PackageReference Include="Sentry.AspNetCore" Version="5.0.0" />
         <PackageReference Include="Swashbuckle.AspNetCore" Version="7.1.0" />
         <PackageReference Include="Swashbuckle.AspNetCore.ReDoc" Version="7.1.0" />
     </ItemGroup>