diff --git a/Api.SeaHavenIndustries.Tests/SentryObservabilityTests.cs b/Api.SeaHavenIndustries.Tests/SentryObservabilityTests.cs new file mode 100644 index 0000000..7190018 --- /dev/null +++ b/Api.SeaHavenIndustries.Tests/SentryObservabilityTests.cs @@ -0,0 +1,256 @@ +using System.Security.Claims; +using Api.SeaHavenIndustries.Observability; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; +using Moq; +using Sentry; +using Xunit; + +namespace Api.SeaHavenIndustries.Tests; + +public class SentryObservabilityTests +{ + private static readonly string ValidSha = new string('a', 40); + + [Fact] + public void ResolveRelease_Accepts_ServicePrefixed40HexCommit() + { + SentryObservability.ResolveRelease("shoc-backend@" + ValidSha) + .Should().Be("shoc-backend@" + ValidSha); + } + + [Theory] + [InlineData("1.0.0")] + [InlineData("shoc-backend@abc")] + [InlineData("shoc-backend@not-a-sha-at-all")] + [InlineData("other-service@" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")] + [InlineData("")] + public void ResolveRelease_FallsBackToLocalDevelopment_ForNonReleaseVersion(string version) + { + SentryObservability.ResolveRelease(version) + .Should().Be(SentryObservability.LocalDevelopmentRelease); + } + + [Fact] + public void ResolveCommitSha_ReturnsSha_WhenReleaseShaped() + { + var release = SentryObservability.ResolveRelease("shoc-backend@" + ValidSha); + release[(SentryObservability.ServiceName.Length + 1)..].Should().Be(ValidSha); + } + + [Fact] + public void ConfigureRequest_SetsUser_FromNameIdentifierOnly_AndUsesRouteTemplateNotValues() + { + var (hub, scope) = CreateHub(); + var context = CreateHttpContext( + routePattern: "/api/work-orders/{workOrderId}", + claims: new[] + { + new Claim(ClaimTypes.NameIdentifier, "user-123"), + new Claim(ClaimTypes.Email, "secret.user@example.com"), + new Claim(ClaimTypes.Name, "Secret User"), + }); + context.Request.QueryString = new QueryString("?ticket=secret-query-value"); + context.Request.Method = "GET"; + + SentryObservability.ConfigureRequest(hub.Object, context); + + scope.User.Should().NotBeNull(); + scope.User!.Id.Should().Be("user-123"); + scope.User.Email.Should().BeNull(); + scope.User.Username.Should().BeNull(); + + scope.Tags["actor.type"].Should().Be("authenticated"); + scope.Tags["operation.type"].Should().Be("http.server"); + scope.Tags["code.function"].Should().Be("WorkOrders.GetWorkOrder"); + scope.Tags["http.method"].Should().Be("GET"); + scope.Tags["http.route"].Should().Be("/api/work-orders/{workOrderId}"); + scope.Tags["trace_id"].Should().NotBeNullOrWhiteSpace(); + scope.Tags["transaction_id"].Should().NotBeNullOrWhiteSpace(); + + scope.Tags.Values.Should().NotContain(value => value != null && value.Contains("secret", StringComparison.OrdinalIgnoreCase), + "no query, header, body, cookie, name, or email material may reach Sentry tags"); + } + + [Fact] + public void ConfigureRequest_AnonymousRequest_TagsAnonymous_AndSetsNoUser() + { + var (hub, scope) = CreateHub(); + var context = CreateHttpContext("/api/vendors", Array.Empty()); + + SentryObservability.ConfigureRequest(hub.Object, context); + + scope.User.Id.Should().BeNull(); + scope.Tags["actor.type"].Should().Be("anonymous"); + scope.Tags["code.function"].Should().Be("Vendors.List"); + scope.Tags["http.route"].Should().Be("/api/vendors"); + } + + [Fact] + public void ConfigureRequest_NonControllerEndpoint_OmitsCodeFunction() + { + var (hub, scope) = CreateHub(); + var context = CreateHttpContext("/health", Array.Empty(), includeActionDescriptor: false); + + SentryObservability.ConfigureRequest(hub.Object, context); + + scope.Tags.Should().NotContainKey("code.function"); + } + + [Fact] + public void BeginBackgroundTransaction_SetsScopeTransaction_AndSystemTags() + { + var (hub, scope) = CreateHub(); + var transaction = new Mock(); + hub.Setup(h => h.StartTransaction( + It.IsAny(), + It.IsAny>())) + .Returns(transaction.Object); + + using var handle = SentryObservability.BeginBackgroundTransaction( + hub.Object, + "workorders.job", + "MyWorker.Run", + "the-reason"); + + hub.Verify(h => h.StartTransaction( + It.Is(context => context.Name == "workorders.job" && context.Operation == "task"), + It.IsAny>()), Times.Once); + scope.Transaction.Should().BeSameAs(transaction.Object); + transaction.VerifySet(t => t.Description = "the-reason", Times.Once); + + transaction.Verify(t => t.SetTag("actor.type", "system"), Times.Once); + transaction.Verify(t => t.SetTag("operation.type", "background_job"), Times.Once); + transaction.Verify(t => t.SetTag("code.function", "MyWorker.Run"), Times.Once); + transaction.Verify(t => t.SetTag("trace_id", It.IsAny()), Times.Once); + transaction.Verify(t => t.SetTag("transaction_id", It.IsAny()), Times.Once); + scope.Tags["operation.type"].Should().Be("background_job"); + } + + [Fact] + public void BeginBackgroundTransaction_Handle_CannotDoubleFinish() + { + var (hub, _) = CreateHub(); + var transaction = new Mock(); + hub.Setup(h => h.StartTransaction( + It.IsAny(), + It.IsAny>())) + .Returns(transaction.Object); + + using var handle = SentryObservability.BeginBackgroundTransaction(hub.Object, "job", "Fn"); + + handle.FinishOk(); + handle.FinishOk(); + handle.FinishError(new InvalidOperationException("late")); + handle.FinishCancelled(); + + transaction.Verify(t => t.Finish(SpanStatus.Ok), Times.Once); + transaction.Verify(t => t.Finish(It.IsAny(), It.IsAny()), Times.Never); + transaction.Verify(t => t.Finish(SpanStatus.Cancelled), Times.Never); + } + + [Fact] + public void TelemetryScrubber_RemovesRequestSecrets_AndKeepsOnlyOpaqueUserId() + { + var @event = new SentryEvent + { + User = new SentryUser + { + Id = "opaque-user-123", + Email = "secret.user@example.com", + Username = "Secret User", + IpAddress = "203.0.113.10", + }, + Request = new SentryRequest + { + Data = "secret-body", + QueryString = "token=secret-query", + Cookies = "session=secret-cookie", + }, + }; + @event.Request.Headers["Authorization"] = "Bearer secret-token"; + @event.Request.Env["private"] = "secret-env"; + @event.Request.Other["private"] = "secret-other"; + + var scrubbed = SentryTelemetryScrubber.Scrub(@event); + + scrubbed.Request!.Data.Should().BeNull(); + scrubbed.Request.QueryString.Should().BeNull(); + scrubbed.Request.Cookies.Should().BeNull(); + scrubbed.Request.Headers.Should().BeEmpty(); + scrubbed.Request.Env.Should().BeEmpty(); + scrubbed.Request.Other.Should().BeEmpty(); + scrubbed.User.Should().NotBeNull(); + scrubbed.User!.Id.Should().Be("opaque-user-123"); + scrubbed.User.Email.Should().BeNull(); + scrubbed.User.Username.Should().BeNull(); + scrubbed.User.IpAddress.Should().BeNull(); + } + + [Fact] + public void TelemetryScrubber_RemovesSensitiveTransactionData() + { + var transaction = new SentryTransaction("GET /api/work-orders/{id}", "http.server"); + transaction.SetData("http.route", "/api/work-orders/{id}"); + transaction.SetData("http.query", "token=secret-query"); +#pragma warning disable CS0618 // Exercise the scrubber's legacy Extra cleanup contract. + transaction.SetExtra("secret", "secret-extra"); +#pragma warning restore CS0618 + var scrubbed = SentryTelemetryScrubber.Scrub(transaction); + + scrubbed.Data.Should().ContainKey("http.route"); + scrubbed.Data.Should().NotContainKey("http.query"); +#pragma warning disable CS0618 // Exercise the scrubber's legacy Extra cleanup contract. + scrubbed.Extra.Should().NotContainKey("secret"); +#pragma warning restore CS0618 + scrubbed.Tags["trace_id"].Should().Be(scrubbed.TraceId.ToString()); + scrubbed.Tags["transaction_id"].Should().Be(scrubbed.SpanId.ToString()); + } + + private static (Mock Hub, Scope Scope) CreateHub() + { + var hub = new Mock(); + var scope = new Scope(new SentryOptions()); + hub.Setup(h => h.ConfigureScope(It.IsAny>())) + .Callback>(action => action(scope)); + hub.Setup(h => h.PushScope()).Returns(Mock.Of()); + + var activeSpan = new Mock(); + activeSpan.SetupGet(span => span.TraceId).Returns(SentryId.Create()); + activeSpan.SetupGet(span => span.SpanId).Returns(SpanId.Create()); + hub.Setup(h => h.GetSpan()).Returns(activeSpan.Object); + return (hub, scope); + } + + private static HttpContext CreateHttpContext( + string routePattern, + Claim[] claims, + bool includeActionDescriptor = true) + { + var metadata = includeActionDescriptor + ? new object[] + { + new ControllerActionDescriptor + { + ControllerName = routePattern.StartsWith("/api/work-orders") ? "WorkOrders" : "Vendors", + ActionName = routePattern.StartsWith("/api/work-orders") ? "GetWorkOrder" : "List", + }, + } + : Array.Empty(); + + var endpoint = new RouteEndpoint( + _ => Task.CompletedTask, + RoutePatternFactory.Parse(routePattern), + 0, + new EndpointMetadataCollection(metadata), + "test-endpoint"); + + var context = new DefaultHttpContext(); + context.SetEndpoint(endpoint); + context.User = new ClaimsPrincipal(new ClaimsIdentity(claims, "TestAuth")); + return context; + } +} diff --git a/Api.SeaHavenIndustries.Tests/SentryPipelineContractTests.cs b/Api.SeaHavenIndustries.Tests/SentryPipelineContractTests.cs new file mode 100644 index 0000000..b5ff0d3 --- /dev/null +++ b/Api.SeaHavenIndustries.Tests/SentryPipelineContractTests.cs @@ -0,0 +1,72 @@ +using FluentAssertions; +using Xunit; + +namespace Api.SeaHavenIndustries.Tests; + +public class SentryPipelineContractTests +{ + private static string RepoRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory is not null + && !File.Exists(Path.Combine(directory.FullName, "Api.SeaHavenIndustries", "Program.cs"))) + { + directory = directory.Parent; + } + + if (directory is null) + throw new InvalidOperationException("Could not locate the repository root from " + AppContext.BaseDirectory); + + return directory.FullName; + } + + [Fact] + public void Program_MiddlewareOrder_IsRouting_Authentication_SentryMetadata_Authorization() + { + var program = File.ReadAllText(Path.Combine(RepoRoot(), "Api.SeaHavenIndustries", "Program.cs")); + + var routing = program.IndexOf("app.UseRouting()", StringComparison.Ordinal); + var authentication = program.IndexOf("app.UseAuthentication()", StringComparison.Ordinal); + var sentry = program.IndexOf("app.UseMiddleware()", StringComparison.Ordinal); + var authorization = program.IndexOf("app.UseAuthorization()", StringComparison.Ordinal); + + routing.Should().BeGreaterThan(-1, "UseRouting must be explicit"); + authentication.Should().BeGreaterThan(-1); + sentry.Should().BeGreaterThan(-1); + authorization.Should().BeGreaterThan(-1); + + routing.Should().BeLessThan(authentication, "routing must precede authentication"); + authentication.Should().BeLessThan(sentry, "Sentry request metadata must be applied after authentication"); + sentry.Should().BeLessThan(authorization, "Sentry request metadata must be applied before authorization"); + } + + [Fact] + public void Program_ConfiguresReleaseAndDefaultTags() + { + var program = File.ReadAllText(Path.Combine(RepoRoot(), "Api.SeaHavenIndustries", "Program.cs")); + + program.Should().Contain("SentryObservability.ResolveRelease"); + program.Should().Contain("options.Release = release"); + program.Should().Contain("\"service\", SentryObservability.ServiceName"); + program.Should().Contain("\"app.commit\""); + program.Should().Contain("MaxRequestBodySize = RequestSize.None"); + program.Should().Contain("SetBeforeSend(SentryTelemetryScrubber.Scrub)"); + program.Should().Contain("SetBeforeSendTransaction(SentryTelemetryScrubber.Scrub)"); + } + + [Theory] + [InlineData("Api.SeaHavenIndustries/Helper/VendorDocumentScanWorker.cs", "vendor-documents.scan-pending")] + [InlineData("Api.SeaHavenIndustries/HostedServices/WorkOrderReconciliationHostedService.cs", "workorders.reconciliation-trigger")] + [InlineData("Api.SeaHavenIndustries/HostedServices/WorkOrderReconciliationHostedService.cs", "workorders.reconciliation-run-pending")] + [InlineData("Api.SeaHavenIndustries/HostedServices/WorkOrderWeekRolledHostedService.cs", "workorders.week-rolled-job")] + [InlineData("Api.SeaHavenIndustries/HostedServices/PastDueCacheHostedService.cs", "workorders.past-due-cache-job")] + [InlineData("Api.SeaHavenIndustries/HostedServices/UpliftLifecycleHostedService.cs", "uplifts.lifecycle-sweep")] + public void Workers_UseSharedBackgroundTransactionHelper_WithStableNames(string relativePath, string transactionName) + { + var source = File.ReadAllText(Path.Combine(RepoRoot(), relativePath)); + + source.Should().Contain($"BeginBackgroundTransaction", "worker transactions must go through the shared helper"); + source.Should().Contain($"\"{transactionName}\""); + source.Should().NotContain("PushScope", "scope pushing is owned by the shared helper"); + } +} diff --git a/Api.SeaHavenIndustries/Helper/VendorDocumentScanWorker.cs b/Api.SeaHavenIndustries/Helper/VendorDocumentScanWorker.cs index c236ab5..b0ac5fd 100644 --- a/Api.SeaHavenIndustries/Helper/VendorDocumentScanWorker.cs +++ b/Api.SeaHavenIndustries/Helper/VendorDocumentScanWorker.cs @@ -1,3 +1,4 @@ +using Api.SeaHavenIndustries.Observability; using Data.SeaHavenIndustries; using Microsoft.EntityFrameworkCore; using Sentry; @@ -34,9 +35,10 @@ namespace Api.SeaHavenIndustries.Helper private async Task ScanPendingAsync(CancellationToken cancellationToken) { - using var sentryScope = _sentryHub.PushScope(); - var transaction = _sentryHub.StartTransaction("vendor-documents.scan-pending", "task"); - _sentryHub.ConfigureScope(scope => scope.Transaction = transaction); + using var transaction = SentryObservability.BeginBackgroundTransaction( + _sentryHub, + "vendor-documents.scan-pending", + $"{nameof(VendorDocumentScanWorker)}.{nameof(ScanPendingAsync)}"); try { using var scope = _scopeFactory.CreateScope(); @@ -71,16 +73,16 @@ namespace Api.SeaHavenIndustries.Helper File.Delete(path); } - transaction.Finish(SpanStatus.Ok); + transaction.FinishOk(); } catch (OperationCanceledException) { - transaction.Finish(SpanStatus.Cancelled); + transaction.FinishCancelled(); throw; } catch (Exception ex) { - transaction.Finish(ex, SpanStatus.InternalError); + transaction.FinishError(ex); throw; } } diff --git a/Api.SeaHavenIndustries/HostedServices/PastDueCacheHostedService.cs b/Api.SeaHavenIndustries/HostedServices/PastDueCacheHostedService.cs index 48e2de7..a78f326 100644 --- a/Api.SeaHavenIndustries/HostedServices/PastDueCacheHostedService.cs +++ b/Api.SeaHavenIndustries/HostedServices/PastDueCacheHostedService.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Options; using SeaHaven.Services.Implementation; using SeaHaven.Services.Interfaces; using Api.SeaHavenIndustries.Options; +using Api.SeaHavenIndustries.Observability; using Sentry; namespace Api.SeaHavenIndustries.HostedServices @@ -49,9 +50,10 @@ namespace Api.SeaHavenIndustries.HostedServices private async Task RunJobAsync(CancellationToken stoppingToken) { - using var sentryScope = _sentryHub.PushScope(); - var transaction = _sentryHub.StartTransaction("workorders.past-due-cache-job", "task"); - _sentryHub.ConfigureScope(scope => scope.Transaction = transaction); + using var transaction = SentryObservability.BeginBackgroundTransaction( + _sentryHub, + "workorders.past-due-cache-job", + $"{nameof(PastDueCacheHostedService)}.{nameof(RunJobAsync)}"); try { using var scope = _scopeFactory.CreateScope(); @@ -60,19 +62,19 @@ namespace Api.SeaHavenIndustries.HostedServices _runState.LastPastDueCacheRunUtc = DateTime.UtcNow; _runState.LastPastDueCacheError = null; - transaction.Finish(SpanStatus.Ok); + transaction.FinishOk(); return true; } catch (OperationCanceledException) { - transaction.Finish(SpanStatus.Cancelled); + transaction.FinishCancelled(); throw; } catch (Exception ex) when (ex is not OperationCanceledException) { _runState.LastPastDueCacheError = ex.Message; _logger.LogError(ex, "PastDue cache hosted job failed."); - transaction.Finish(ex, SpanStatus.InternalError); + transaction.FinishError(ex); return false; } } diff --git a/Api.SeaHavenIndustries/HostedServices/UpliftLifecycleHostedService.cs b/Api.SeaHavenIndustries/HostedServices/UpliftLifecycleHostedService.cs index d4de208..fdebb0b 100644 --- a/Api.SeaHavenIndustries/HostedServices/UpliftLifecycleHostedService.cs +++ b/Api.SeaHavenIndustries/HostedServices/UpliftLifecycleHostedService.cs @@ -1,3 +1,4 @@ +using Api.SeaHavenIndustries.Observability; using Microsoft.Extensions.Options; using SeaHaven.Services.Configuration; using SeaHaven.Services.Interfaces; @@ -29,23 +30,24 @@ namespace Api.SeaHavenIndustries.HostedServices while (!stoppingToken.IsCancellationRequested) { { - using var sentryScope = _sentryHub.PushScope(); - var transaction = _sentryHub.StartTransaction("uplifts.lifecycle-sweep", "task"); - _sentryHub.ConfigureScope(scope => scope.Transaction = transaction); + using var transaction = SentryObservability.BeginBackgroundTransaction( + _sentryHub, + "uplifts.lifecycle-sweep", + $"{nameof(UpliftLifecycleHostedService)}.{nameof(SweepAsync)}"); try { await SweepAsync(stoppingToken); - transaction.Finish(SpanStatus.Ok); + transaction.FinishOk(); } catch (OperationCanceledException) { - transaction.Finish(SpanStatus.Cancelled); + transaction.FinishCancelled(); throw; } catch (Exception ex) { _logger.LogError(ex, "Uplift lifecycle sweep failed; will retry on next interval"); - transaction.Finish(ex, SpanStatus.InternalError); + transaction.FinishError(ex); } } diff --git a/Api.SeaHavenIndustries/HostedServices/WorkOrderReconciliationHostedService.cs b/Api.SeaHavenIndustries/HostedServices/WorkOrderReconciliationHostedService.cs index 0f74200..4fdd065 100644 --- a/Api.SeaHavenIndustries/HostedServices/WorkOrderReconciliationHostedService.cs +++ b/Api.SeaHavenIndustries/HostedServices/WorkOrderReconciliationHostedService.cs @@ -1,3 +1,4 @@ +using Api.SeaHavenIndustries.Observability; using Microsoft.Extensions.Options; using SeaHaven.Services.Configuration; using SeaHaven.Services.Interfaces; @@ -60,50 +61,52 @@ namespace Api.SeaHavenIndustries.HostedServices private async Task TriggerAsync(string reason, CancellationToken cancellationToken) { - using var sentryScope = _sentryHub.PushScope(); - var transaction = _sentryHub.StartTransaction("workorders.reconciliation-trigger", "task"); - transaction.Description = reason; - _sentryHub.ConfigureScope(scope => scope.Transaction = transaction); + using var transaction = SentryObservability.BeginBackgroundTransaction( + _sentryHub, + "workorders.reconciliation-trigger", + $"{nameof(WorkOrderReconciliationHostedService)}.{nameof(TriggerAsync)}", + description: reason); try { await using var scope = _scopeFactory.CreateAsyncScope(); var runner = scope.ServiceProvider.GetRequiredService(); await runner.TriggerAsync(reason, cancellationToken); - transaction.Finish(SpanStatus.Ok); + transaction.FinishOk(); } catch (OperationCanceledException) { - transaction.Finish(SpanStatus.Cancelled); + transaction.FinishCancelled(); throw; } catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Could not queue procurement reconciliation."); - transaction.Finish(ex, SpanStatus.InternalError); + transaction.FinishError(ex); } } private async Task RunPendingAsync(CancellationToken cancellationToken) { - using var sentryScope = _sentryHub.PushScope(); - var transaction = _sentryHub.StartTransaction("workorders.reconciliation-run-pending", "task"); - _sentryHub.ConfigureScope(scope => scope.Transaction = transaction); + using var transaction = SentryObservability.BeginBackgroundTransaction( + _sentryHub, + "workorders.reconciliation-run-pending", + $"{nameof(WorkOrderReconciliationHostedService)}.{nameof(RunPendingAsync)}"); try { await using var scope = _scopeFactory.CreateAsyncScope(); var runner = scope.ServiceProvider.GetRequiredService(); await runner.RunPendingAsync(cancellationToken); - transaction.Finish(SpanStatus.Ok); + transaction.FinishOk(); } catch (OperationCanceledException) { - transaction.Finish(SpanStatus.Cancelled); + transaction.FinishCancelled(); throw; } catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Could not execute procurement reconciliation."); - transaction.Finish(ex, SpanStatus.InternalError); + transaction.FinishError(ex); } } } diff --git a/Api.SeaHavenIndustries/HostedServices/WorkOrderWeekRolledHostedService.cs b/Api.SeaHavenIndustries/HostedServices/WorkOrderWeekRolledHostedService.cs index 0ac73ae..b3472a3 100644 --- a/Api.SeaHavenIndustries/HostedServices/WorkOrderWeekRolledHostedService.cs +++ b/Api.SeaHavenIndustries/HostedServices/WorkOrderWeekRolledHostedService.cs @@ -3,6 +3,7 @@ using SeaHaven.Services.Helpers; using SeaHaven.Services.Implementation; using SeaHaven.Services.Interfaces; using Api.SeaHavenIndustries.Options; +using Api.SeaHavenIndustries.Observability; using Sentry; namespace Api.SeaHavenIndustries.HostedServices @@ -50,9 +51,10 @@ namespace Api.SeaHavenIndustries.HostedServices private async Task RunJobAsync(CancellationToken stoppingToken) { - using var sentryScope = _sentryHub.PushScope(); - var transaction = _sentryHub.StartTransaction("workorders.week-rolled-job", "task"); - _sentryHub.ConfigureScope(scope => scope.Transaction = transaction); + using var transaction = SentryObservability.BeginBackgroundTransaction( + _sentryHub, + "workorders.week-rolled-job", + $"{nameof(WorkOrderWeekRolledHostedService)}.{nameof(RunJobAsync)}"); try { var sourceWeekStart = WorkOrderOperationalWeek.GetPreviousOperationalWeekStart(DateTime.UtcNow); @@ -63,19 +65,19 @@ namespace Api.SeaHavenIndustries.HostedServices _runState.LastWeekRolledRunUtc = DateTime.UtcNow; _runState.LastWeekRolledError = null; - transaction.Finish(SpanStatus.Ok); + transaction.FinishOk(); return true; } catch (OperationCanceledException) { - transaction.Finish(SpanStatus.Cancelled); + transaction.FinishCancelled(); throw; } catch (Exception ex) when (ex is not OperationCanceledException) { _runState.LastWeekRolledError = ex.Message; _logger.LogError(ex, "WeekRolled hosted job failed."); - transaction.Finish(ex, SpanStatus.InternalError); + transaction.FinishError(ex); return false; } } diff --git a/Api.SeaHavenIndustries/Middleware/SentryRequestMetadataMiddleware.cs b/Api.SeaHavenIndustries/Middleware/SentryRequestMetadataMiddleware.cs new file mode 100644 index 0000000..85b40cc --- /dev/null +++ b/Api.SeaHavenIndustries/Middleware/SentryRequestMetadataMiddleware.cs @@ -0,0 +1,20 @@ +using Api.SeaHavenIndustries.Observability; +using Sentry; + +namespace Api.SeaHavenIndustries.Middleware; + +public sealed class SentryRequestMetadataMiddleware +{ + private readonly RequestDelegate _next; + + public SentryRequestMetadataMiddleware(RequestDelegate next) + { + _next = next; + } + + public async Task InvokeAsync(HttpContext context, IHub sentryHub) + { + SentryObservability.ConfigureRequest(sentryHub, context); + await _next(context); + } +} diff --git a/Api.SeaHavenIndustries/Observability/SentryObservability.cs b/Api.SeaHavenIndustries/Observability/SentryObservability.cs new file mode 100644 index 0000000..fbb6604 --- /dev/null +++ b/Api.SeaHavenIndustries/Observability/SentryObservability.cs @@ -0,0 +1,268 @@ +using System.Reflection; +using System.Security.Claims; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Routing; +using Sentry; + +namespace Api.SeaHavenIndustries.Observability; + +public static class SentryObservability +{ + public const string ServiceName = "shoc-backend"; + public const string LocalDevelopmentRelease = "local-development"; + private const string ReleasePrefix = ServiceName + "@"; + private const int CommitShaLength = 40; + + public static string ResolveRelease(Assembly? assembly) + { + var informationalVersion = assembly? + .GetCustomAttribute()? + .InformationalVersion; + + return ResolveRelease(informationalVersion); + } + + public static string? ResolveCommitSha(Assembly? assembly) + { + var informationalVersion = assembly? + .GetCustomAttribute()? + .InformationalVersion; + + return IsReleaseInformationalVersion(informationalVersion) + ? informationalVersion![ReleasePrefix.Length..] + : null; + } + + public static string ResolveRelease(string? informationalVersion) => + IsReleaseInformationalVersion(informationalVersion) + ? informationalVersion! + : LocalDevelopmentRelease; + + public static bool IsReleaseInformationalVersion(string? value) => + value is not null + && value.StartsWith(ReleasePrefix, StringComparison.Ordinal) + && value.Length == ReleasePrefix.Length + CommitShaLength + && value[ReleasePrefix.Length..].All(c => c is >= '0' and <= '9' or >= 'a' and <= 'f' or >= 'A' and <= 'F'); + + public static void ConfigureRequest(IHub hub, HttpContext context) + { + var endpoint = context.GetEndpoint(); + var routeEndpoint = endpoint as RouteEndpoint; + var routeTemplate = routeEndpoint?.RoutePattern.RawText; + + var actionDescriptor = endpoint?.Metadata.GetMetadata(); + var userId = context.User.FindFirst(ClaimTypes.NameIdentifier)?.Value; + var authenticated = !string.IsNullOrEmpty(userId); + + hub.ConfigureScope(scope => + { + if (authenticated) + scope.User = new SentryUser { Id = userId }; + + scope.SetTag("actor.type", authenticated ? "authenticated" : "anonymous"); + scope.SetTag("operation.type", "http.server"); + + if (actionDescriptor is not null) + scope.SetTag("code.function", $"{actionDescriptor.ControllerName}.{actionDescriptor.ActionName}"); + + scope.SetTag("http.method", context.Request.Method); + + if (!string.IsNullOrEmpty(routeTemplate)) + scope.SetTag("http.route", routeTemplate); + + var activeSpan = hub.GetSpan(); + if (activeSpan is not null) + { + scope.SetTag("trace_id", activeSpan.TraceId.ToString()); + scope.SetTag("transaction_id", activeSpan.SpanId.ToString()); + } + }); + } + + public static SentryBackgroundTransactionHandle BeginBackgroundTransaction( + IHub hub, + string name, + string function, + string? description = null) + { + var scope = hub.PushScope(); + var transaction = hub.StartTransaction(name, "task"); + + if (!string.IsNullOrEmpty(description)) + transaction.Description = description; + + hub.ConfigureScope(s => s.Transaction = transaction); + + foreach (var (key, value) in new[] + { + ("actor.type", "system"), + ("operation.type", "background_job"), + ("code.function", function), + ("trace_id", transaction.TraceId.ToString()), + ("transaction_id", transaction.SpanId.ToString()), + }) + { + transaction.SetTag(key, value); + hub.ConfigureScope(s => s.SetTag(key, value)); + } + + return new SentryBackgroundTransactionHandle(transaction, scope); + } +} + +public static class SentryTelemetryScrubber +{ + private static readonly HashSet SafeSpanDataKeys = new(StringComparer.Ordinal) + { + "actor.type", + "code.function", + "db.operation.name", + "db.system", + "http.method", + "http.request.method", + "http.response.status_code", + "http.route", + "http.status_code", + "network.protocol.version", + "operation.type", + "server.address", + "url.scheme", + }; + + private static readonly HashSet SafeSpanTagKeys = new(StringComparer.Ordinal) + { + "actor.type", + "code.function", + "http.method", + "http.route", + "operation.type", + }; + + public static SentryEvent Scrub(SentryEvent @event) + { + ScrubRequest(@event.Request); + @event.User = KeepOpaqueIdOnly(@event.User); + return @event; + } + + public static SentryTransaction Scrub(SentryTransaction transaction) + { + ScrubRequest(transaction.Request); + transaction.User = KeepOpaqueIdOnly(transaction.User); + transaction.SetTag("trace_id", transaction.TraceId.ToString()); + transaction.SetTag("transaction_id", transaction.SpanId.ToString()); + ScrubDictionary(transaction.Data, SafeSpanDataKeys); + + foreach (var span in transaction.Spans) + { + ScrubDictionary(span.Data, SafeSpanDataKeys); + + foreach (var tag in span.Tags.Keys.Where(key => !SafeSpanTagKeys.Contains(key)).ToArray()) + span.UnsetTag(tag); + + span.Description = span.Operation?.Contains("http", StringComparison.OrdinalIgnoreCase) == true + ? NormalizeHttpDescription(span.Description) + : null; + } + + return transaction; + } + + private static SentryUser KeepOpaqueIdOnly(SentryUser? user) => + string.IsNullOrWhiteSpace(user?.Id) ? new SentryUser() : new SentryUser { Id = user.Id }; + + private static void ScrubRequest(SentryRequest? request) + { + if (request is null) + return; + + request.Data = null; + request.QueryString = null; + request.Cookies = null; + request.Headers.Clear(); + request.Env.Clear(); + request.Other.Clear(); + } + + private static void ScrubDictionary( + IReadOnlyDictionary values, + IReadOnlySet allowedKeys) + { + if (values is not IDictionary mutable) + return; + + foreach (var key in mutable.Keys.Where(key => !allowedKeys.Contains(key)).ToArray()) + mutable.Remove(key); + } + + private static string? NormalizeHttpDescription(string? description) + { + if (string.IsNullOrWhiteSpace(description)) + return null; + + var separator = description.IndexOf(' '); + if (separator <= 0 || separator == description.Length - 1) + return null; + + var method = description[..separator].ToUpperInvariant(); + if (method is not ("GET" or "POST" or "PUT" or "PATCH" or "DELETE" or "HEAD" or "OPTIONS")) + return null; + + var rawUrl = description[(separator + 1)..]; + var path = Uri.TryCreate(rawUrl, UriKind.Absolute, out var absolute) + ? absolute.GetLeftPart(UriPartial.Authority) + absolute.AbsolutePath + : rawUrl.Split('?', '#')[0]; + + return $"{method} {NormalizePathIdentifiers(path)}"; + } + + private static string NormalizePathIdentifiers(string path) => + string.Join('/', path.Split('/').Select(segment => IsIdentifierSegment(segment) ? ":id" : segment)); + + private static bool IsIdentifierSegment(string segment) + { + if (string.IsNullOrEmpty(segment)) + return false; + + if (long.TryParse(segment, out _) || Guid.TryParse(segment, out _)) + return true; + + var allHex = segment.All(Uri.IsHexDigit); + if (segment.Length >= 8 && allHex) + return true; + + return segment.Length >= 16 + && segment.All(c => char.IsAsciiLetterOrDigit(c) || c is '.' or '_' or '~' or '-') + && segment.Any(char.IsAsciiLetter) + && segment.Any(char.IsAsciiDigit); + } +} + +public sealed class SentryBackgroundTransactionHandle : IDisposable +{ + private readonly ITransactionTracer _transaction; + private readonly IDisposable _scope; + private int _finished; + + internal SentryBackgroundTransactionHandle(ITransactionTracer transaction, IDisposable scope) + { + _transaction = transaction; + _scope = scope; + } + + public void FinishOk() => Finish(() => _transaction.Finish(SpanStatus.Ok)); + + public void FinishCancelled() => Finish(() => _transaction.Finish(SpanStatus.Cancelled)); + + public void FinishError(Exception exception) => + Finish(() => _transaction.Finish(exception, SpanStatus.InternalError)); + + private void Finish(Action finish) + { + if (Interlocked.Exchange(ref _finished, 1) == 0) + finish(); + } + + public void Dispose() => _scope.Dispose(); +} diff --git a/Api.SeaHavenIndustries/Program.cs b/Api.SeaHavenIndustries/Program.cs index 87bc2a3..f220dfa 100644 --- a/Api.SeaHavenIndustries/Program.cs +++ b/Api.SeaHavenIndustries/Program.cs @@ -1,6 +1,7 @@ using Api.SeaHavenIndustries.Helper; using Api.SeaHavenIndustries.HostedServices; using Api.SeaHavenIndustries.Middleware; +using Api.SeaHavenIndustries.Observability; using Api.SeaHavenIndustries.Options; using Data.SeaHavenIndustries; using Microsoft.AspNetCore.Authentication.JwtBearer; @@ -9,6 +10,8 @@ using Microsoft.AspNetCore.ResponseCompression; using Microsoft.EntityFrameworkCore; using Microsoft.IdentityModel.Tokens; using Microsoft.OpenApi.Models; +using Sentry.AspNetCore; +using Sentry.Extensibility; using System.Text; using SeaHaven.DataServices.DependencyInjection; using SeaHaven.Services.DependencyInjection; @@ -17,13 +20,23 @@ using SeaHaven.Services.Interfaces; var builder = WebApplication.CreateBuilder(args); -builder.WebHost.UseSentry(options => +var entryAssembly = System.Reflection.Assembly.GetEntryAssembly(); +var release = SentryObservability.ResolveRelease(entryAssembly); +var commitSha = SentryObservability.ResolveCommitSha(entryAssembly); + +builder.WebHost.UseSentry((SentryAspNetCoreOptions options) => { options.Dsn = builder.Configuration["SENTRY_DSN"] ?? string.Empty; options.Environment = builder.Configuration["SENTRY_ENVIRONMENT"] ?? builder.Environment.EnvironmentName.ToLowerInvariant(); + options.Release = release; + options.DefaultTags.Add("service", SentryObservability.ServiceName); + options.DefaultTags.Add("app.commit", commitSha ?? SentryObservability.LocalDevelopmentRelease); options.TracesSampleRate = 1.0; options.SendDefaultPii = false; + options.MaxRequestBodySize = RequestSize.None; + options.SetBeforeSend(SentryTelemetryScrubber.Scrub); + options.SetBeforeSendTransaction(SentryTelemetryScrubber.Scrub); }); ConfigurationManager configuration = builder.Configuration; @@ -202,7 +215,9 @@ if (!app.Environment.IsDevelopment()) app.UseStaticFiles(); app.UseCors(); app.UseMiddleware(); +app.UseRouting(); app.UseAuthentication(); +app.UseMiddleware(); app.UseAuthorization(); app.MapControllers(); diff --git a/docs/adr/0002-sentry-observability.md b/docs/adr/0002-sentry-observability.md new file mode 100644 index 0000000..6bf7c52 --- /dev/null +++ b/docs/adr/0002-sentry-observability.md @@ -0,0 +1,76 @@ +# ADR 0002 — Sentry observability: release identity and redaction-safe telemetry fields + +- Status: Accepted +- Date: 2026-09-03 +- Scope: `Api.SeaHavenIndustries` (observability layer only; no behavior changes) + +## Context + +The backend reports to Sentry (SH-298). Events arrived without a stable release +identity, without service/commit attribution, and background jobs each hand-rolled +their own transaction setup. HTTP request metadata risked leaking PII through +route values, query strings, or identity claims. + +## Decision + +1. **Release identity.** `SentryObservability.ResolveRelease(Assembly)` accepts an + informational version shaped `shoc-backend@<40 hex commit sha>` and otherwise + resolves to `local-development`. `scripts/package-elastic-beanstalk.sh` resolves + the commit from `APP_COMMIT_SHA` → `GITHUB_SHA` → `git rev-parse HEAD`, fails + when `CI=true` cannot produce a valid 40-hex sha, and publishes with + `-p:SourceRevisionId`, `-p:InformationalVersion=shoc-backend@`, + `-p:DebugType=portable`, `-p:DebugSymbols=true`. +2. **Default tags.** `service=shoc-backend` and `app.commit=` + on `SentryOptions.DefaultTags`. All events use the same commit-addressed + `shoc-backend@` release. +3. **HTTP requests.** `SentryRequestMetadataMiddleware` runs **after** + routing/authentication and **before** authorization. It sets on the current + scope only the fields below. Non-controller endpoints omit `code.function`. +4. **Background jobs.** All six worker transaction blocks (five files) use + `SentryObservability.BeginBackgroundTransaction`, which pushes a scope, starts + the transaction, sets the scope transaction, and returns a handle with + `FinishOk` / `FinishCancelled` / `FinishError(Exception)` that cannot + double-finish. Existing status/error behavior is preserved. +5. **Last-mile privacy.** Request-body extraction is disabled. Global before-send + processors clear request data, query strings, cookies, headers, environment + values, and miscellaneous request values from both error events and + transactions, retain only an opaque user ID, and attach native trace and + transaction IDs as searchable transaction tags. Root and child span data use + an allowlist for function, operation, actor, route template, method/status, + protocol, database operation, and service host; raw URLs, query values, + network addresses, and arbitrary extras are removed. + +## Field / redaction contract + +Allowed request fields (template values only — never resolved route values): + +| Field | Source | Rule | +| --- | --- | --- | +| `actor.type` | auth state | `authenticated` / `anonymous` / `system` | +| `operation.type` | static | `http.server` / `background_job` | +| `code.function` | `ControllerActionDescriptor` | `ControllerName.ActionName` only | +| `http.method` | request | HTTP verb | +| `http.route` | `RouteEndpoint.RoutePattern.RawText` | route template; omitted when unresolved | +| `trace_id` / `transaction_id` | active Sentry span/transaction | native Sentry ids | +| scope `User.Id` | `ClaimTypes.NameIdentifier` only | set only when authenticated | + +Never intentionally sent: query strings, headers, bodies, cookies, usernames, +emails, IP addresses, route values, credentials, or token values. The application +retains stack traces, exception types/messages, safe tags, operation status, and +route templates because those are required to identify the failing function and +operation. + +## Consequences + +- Release health maps 1:1 to a commit sha; local builds are clearly labeled. +- One redaction contract to audit, enforced by + `SentryObservabilityTests`/`SentryPipelineContractTests`. +- ASP.NET Core request transactions and the SDK's registered outgoing HTTP + instrumentation cover inbound API calls and the platform's typed procurement + client; explicit root transactions cover every hosted worker loop. + +## Alternatives considered + +- Per-worker transaction setup (rejected: duplication drifted across five files). +- Sentry's default request payload with PII scrubbing (rejected: default-deny is + safer than default-capture-then-scrub). diff --git a/scripts/package-elastic-beanstalk.sh b/scripts/package-elastic-beanstalk.sh index 00c94aa..27e1288 100755 --- a/scripts/package-elastic-beanstalk.sh +++ b/scripts/package-elastic-beanstalk.sh @@ -67,6 +67,23 @@ RUNTIME="${RUNTIME:-linux-x64}" [[ -f "$API_PROJECT" ]] || die "missing API project: $API_PROJECT" [[ -f "$MIGRATIONS_PROJECT" ]] || die "missing migrations project: $MIGRATIONS_PROJECT" + +log "resolve commit SHA for release metadata" +COMMIT_SHA="${APP_COMMIT_SHA:-${GITHUB_SHA:-}}" +if [[ ! "$COMMIT_SHA" =~ ^[0-9a-fA-F]{40}$ ]] && command -v git >/dev/null 2>&1; then + GIT_SHA="$(git rev-parse HEAD 2>/dev/null || true)" + if [[ "$GIT_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + COMMIT_SHA="$GIT_SHA" + fi +fi +if [[ ! "$COMMIT_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + if [[ "${CI:-false}" == "true" ]]; then + die "CI build could not resolve a valid 40-hex commit SHA (APP_COMMIT_SHA/GITHUB_SHA/git)." + fi + die "could not resolve a valid 40-hex commit SHA (APP_COMMIT_SHA/GITHUB_SHA/git rev-parse HEAD)." +fi +COMMIT_SHA="$(printf '%s' "$COMMIT_SHA" | tr '[:upper:]' '[:lower:]')" +printf ' commit: %s\n' "$COMMIT_SHA" if command -v zip >/dev/null 2>&1; then ARCHIVER="zip" elif command -v python >/dev/null 2>&1; then @@ -92,7 +109,11 @@ log "publish $API_PROJECT (Release, self-contained, $RUNTIME)" --runtime "$RUNTIME" \ -o "$STAGING_DIR" \ -p:ContinuousIntegrationBuild=true \ - -p:UseAppHost=true + -p:UseAppHost=true \ + -p:SourceRevisionId="$COMMIT_SHA" \ + -p:InformationalVersion="shoc-backend@$COMMIT_SHA" \ + -p:DebugType=portable \ + -p:DebugSymbols=true log "install dotnet-ef $EF_VERSION (local tool path)" if ! "$DOTNET" tool install dotnet-ef --version "$EF_VERSION" --tool-path "$TOOLS_DIR" 2>/dev/null; then