diff --git a/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs b/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs index 2b4e41f47..9c69159f0 100644 --- a/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs +++ b/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs @@ -356,6 +356,15 @@ public async Task StartAsync(CancellationToken cancellationToken) public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; } + /// + /// PDF generation works by using a headless browser to render the frontend of an app instance. + /// To make debugging PDF generation failures easier, we want requests originating from the PDF generator to be + /// contained in the root trace (process/next) as children. The frontend will set this header when making requests to the app backend in PDF mode. + /// + /// Request headers attached to the span + /// + private static bool IsPdfGeneratorRequest(IHeaderDictionary headers) => headers.ContainsKey("X-Altinn-IsPdf"); + internal sealed class OtelPropagator : TextMapPropagator { private readonly TextMapPropagator _inner; @@ -370,8 +379,9 @@ public override PropagationContext Extract( Func?> getter ) { - if (carrier is HttpRequest) + if (carrier is HttpRequest req && !IsPdfGeneratorRequest(req.Headers)) return default; + return _inner.Extract(context, carrier, getter); } @@ -392,7 +402,7 @@ internal sealed class AspNetCorePropagator : DistributedContextPropagator PropagatorGetterCallback? getter ) { - if (carrier is IHeaderDictionary) + if (carrier is IHeaderDictionary headers && !IsPdfGeneratorRequest(headers)) return null; return _inner.ExtractBaggage(carrier, getter); @@ -405,7 +415,7 @@ public override void ExtractTraceIdAndState( out string? traceState ) { - if (carrier is IHeaderDictionary) + if (carrier is IHeaderDictionary headers && !IsPdfGeneratorRequest(headers)) { traceId = null; traceState = null; diff --git a/src/Altinn.App.Core/Features/Telemetry/Telemetry.PdfGeneratorClient.cs b/src/Altinn.App.Core/Features/Telemetry/Telemetry.PdfGeneratorClient.cs new file mode 100644 index 000000000..8066b3c32 --- /dev/null +++ b/src/Altinn.App.Core/Features/Telemetry/Telemetry.PdfGeneratorClient.cs @@ -0,0 +1,12 @@ +using System.Diagnostics; + +namespace Altinn.App.Core.Features; + +partial class Telemetry +{ + internal Activity? StartGeneratePdfClientActivity() + { + var activity = ActivitySource.StartActivity("PdfGeneratorClient.GeneratePdf"); + return activity; + } +} diff --git a/src/Altinn.App.Core/Features/Telemetry/TelemetryActivityExtensions.cs b/src/Altinn.App.Core/Features/Telemetry/TelemetryActivityExtensions.cs index b2c8dab43..7f147c1fa 100644 --- a/src/Altinn.App.Core/Features/Telemetry/TelemetryActivityExtensions.cs +++ b/src/Altinn.App.Core/Features/Telemetry/TelemetryActivityExtensions.cs @@ -344,7 +344,7 @@ internal static Activity SetProblemDetails(this Activity activity, ProblemDetail internal static void Errored(this Activity activity, Exception? exception = null, string? error = null) { activity.SetStatus(ActivityStatusCode.Error, error); - if(exception is not null) + if (exception is not null) { activity.AddException(exception); } diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Pdf/PdfGeneratorClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Pdf/PdfGeneratorClient.cs index 1bdbb0aaa..b525d17c2 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Pdf/PdfGeneratorClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Pdf/PdfGeneratorClient.cs @@ -1,11 +1,15 @@ +using System.Diagnostics; using System.Text; using System.Text.Json; using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features; using Altinn.App.Core.Internal.Auth; using Altinn.App.Core.Internal.Pdf; using Altinn.App.Core.Models.Pdf; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using OpenTelemetry.Context.Propagation; namespace Altinn.App.Core.Infrastructure.Clients.Pdf; @@ -15,20 +19,25 @@ namespace Altinn.App.Core.Infrastructure.Clients.Pdf; /// public class PdfGeneratorClient : IPdfGeneratorClient { + private static readonly TextMapPropagator _w3cPropagator = new TraceContextPropagator(); + private static readonly JsonSerializerOptions _jsonSerializerOptions = new() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, }; + private readonly ILogger _logger; private readonly HttpClient _httpClient; private readonly PdfGeneratorSettings _pdfGeneratorSettings; private readonly PlatformSettings _platformSettings; private readonly IUserTokenProvider _userTokenProvider; private readonly IHttpContextAccessor _httpContextAccessor; + private readonly Telemetry? _telemetry; /// /// Initializes a new instance of the class. /// + /// The logger. /// The HttpClient to use in communication with the PDF generator service. /// /// All generic settings needed for communication with the PDF generator service. @@ -36,19 +45,24 @@ public class PdfGeneratorClient : IPdfGeneratorClient /// Links to platform services /// A service able to identify the JWT for currently authenticated user. /// http context + /// Telemetry service public PdfGeneratorClient( + ILogger logger, HttpClient httpClient, IOptions pdfGeneratorSettings, IOptions platformSettings, IUserTokenProvider userTokenProvider, - IHttpContextAccessor httpContextAccessor + IHttpContextAccessor httpContextAccessor, + Telemetry? telemetry = null ) { + _logger = logger; _httpClient = httpClient; _userTokenProvider = userTokenProvider; _pdfGeneratorSettings = pdfGeneratorSettings.Value; _platformSettings = platformSettings.Value; _httpContextAccessor = httpContextAccessor; + _telemetry = telemetry; } /// @@ -60,6 +74,8 @@ public async Task GeneratePdf(Uri uri, CancellationToken ct) /// public async Task GeneratePdf(Uri uri, string? footerContent, CancellationToken ct) { + using var activity = _telemetry?.StartGeneratePdfClientActivity(); + bool hasWaitForSelector = !string.IsNullOrWhiteSpace(_pdfGeneratorSettings.WaitForSelector); PdfGeneratorRequest generatorRequest = new() { @@ -73,6 +89,33 @@ public async Task GeneratePdf(Uri uri, string? footerContent, Cancellati }, }; + if (Activity.Current is { } propagateActivity) + { + // We want the frontend to attach the current trace context to requests + // when making downstream requests back to the app backend. + // This makes it easier to debug issues (such as slow backend requests during PDF generation). + // The frontend expects to see the "traceparent" and "tracestate" values as cookies (as they are easily propagated). + // It will then pass them back to the backend in the "traceparent" and "tracestate" headers as per W3C spec. + _w3cPropagator.Inject( + new PropagationContext(propagateActivity.Context, default), + generatorRequest.Cookies, + (c, k, v) => + { + if (k != "traceparent" && k != "tracestate") + _logger.LogWarning("Unexpected key '{Key}' when propagating trace context (expected W3C)", k); + + c.Add( + new PdfGeneratorCookieOptions + { + Name = $"altinn-telemetry-{k}", + Value = v, + Domain = uri.Host, + } + ); + } + ); + } + generatorRequest.Cookies.Add( new PdfGeneratorCookieOptions { Value = _userTokenProvider.GetUserToken(), Domain = uri.Host } ); diff --git a/test/Altinn.App.Api.Tests/Controllers/PdfControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/PdfControllerTests.cs index 242477e4a..7625dae95 100644 --- a/test/Altinn.App.Api.Tests/Controllers/PdfControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/PdfControllerTests.cs @@ -83,7 +83,10 @@ public async Task Request_In_Dev_Should_Generate() var handler = new Mock(); var httpClient = new HttpClient(handler.Object); + var logger = new Mock>(); + var pdfGeneratorClient = new PdfGeneratorClient( + logger.Object, httpClient, _pdfGeneratorSettingsOptions, _platformSettingsOptions, @@ -163,7 +166,10 @@ public async Task Request_In_Dev_Should_Include_Frontend_Version() var handler = new Mock(); var httpClient = new HttpClient(handler.Object); + var logger = new Mock>(); + var pdfGeneratorClient = new PdfGeneratorClient( + logger.Object, httpClient, _pdfGeneratorSettingsOptions, _platformSettingsOptions, @@ -245,7 +251,10 @@ public async Task Request_In_TT02_Should_Ignore_Frontend_Version() var handler = new Mock(); var httpClient = new HttpClient(handler.Object); + var logger = new Mock>(); + var pdfGeneratorClient = new PdfGeneratorClient( + logger.Object, httpClient, _pdfGeneratorSettingsOptions, _platformSettingsOptions, diff --git a/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs b/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs index a5ea3272c..95422408e 100644 --- a/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs +++ b/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs @@ -189,6 +189,7 @@ CancellationToken cancellationToken request.Headers, (c, k, v) => c.TryAddWithoutValidation(k, v) ); + Assert.Contains(request.Headers, h => h.Key == "traceparent"); // traceparent is mandatory in W3C } return base.SendAsync(request, cancellationToken); } diff --git a/test/Altinn.App.Api.Tests/Middleware/TelemetryEnrichingMiddlewareTests.Should_Always_Be_A_Root_Trace_Unless_Pdf.verified.txt b/test/Altinn.App.Api.Tests/Middleware/TelemetryEnrichingMiddlewareTests.Should_Always_Be_A_Root_Trace_Unless_Pdf.verified.txt new file mode 100644 index 000000000..c719e047a --- /dev/null +++ b/test/Altinn.App.Api.Tests/Middleware/TelemetryEnrichingMiddlewareTests.Should_Always_Be_A_Root_Trace_Unless_Pdf.verified.txt @@ -0,0 +1,51 @@ +{ + Activities: [ + { + ActivityName: GET {org}/{app}/api/v1/applicationmetadata, + Tags: [ + { + http.request.method: GET + }, + { + http.response.status_code: 200 + }, + { + http.route: {org}/{app}/api/v1/applicationmetadata + }, + { + network.protocol.version: 1.1 + }, + { + server.address: localhost + }, + { + TestId: Guid_1 + }, + { + url.path: /tdd/contributer-restriction/api/v1/applicationmetadata + }, + { + url.scheme: http + }, + { + user.authentication.level: 4 + }, + { + user.authentication.method: Mock + }, + { + user.id: 10 + }, + { + user.name: User10 + }, + { + user.party.id: Scrubbed + } + ], + IdFormat: W3C, + Kind: Server + } + ], + Metrics: [] +} \ No newline at end of file diff --git a/test/Altinn.App.Api.Tests/Middleware/TelemetryEnrichingMiddlewareTests.cs b/test/Altinn.App.Api.Tests/Middleware/TelemetryEnrichingMiddlewareTests.cs index 239efead3..4d825c93b 100644 --- a/test/Altinn.App.Api.Tests/Middleware/TelemetryEnrichingMiddlewareTests.cs +++ b/test/Altinn.App.Api.Tests/Middleware/TelemetryEnrichingMiddlewareTests.cs @@ -17,7 +17,8 @@ public TelemetryEnrichingMiddlewareTests(WebApplicationFactory factory, private (TelemetrySink Telemetry, Func Request) AnalyzeTelemetry( string token, - bool includeTraceContext = false + bool includeTraceContext = false, + bool includePdfHeader = false ) { this.OverrideServicesForThisTest = (services) => @@ -35,6 +36,8 @@ public TelemetryEnrichingMiddlewareTests(WebApplicationFactory factory, var telemetry = this.Services.GetRequiredService(); client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); + if (includePdfHeader) + client.DefaultRequestHeaders.Add("X-Altinn-IsPdf", "true"); return (telemetry, async () => await client.GetStringAsync($"/{org}/{app}/api/v1/applicationmetadata")); } @@ -110,4 +113,32 @@ public async Task Should_Always_Be_A_Root_Trace() await telemetry.Snapshot(activity, c => c.ScrubMember(Telemetry.Labels.UserPartyId)); } + + [Fact] + public async Task Should_Always_Be_A_Root_Trace_Unless_Pdf() + { + var partyId = Random.Shared.Next(); + var principal = PrincipalUtil.GetUserPrincipal(10, partyId, 4); + var token = JwtTokenMock.GenerateToken(principal, new TimeSpan(1, 1, 1)); + + var (telemetry, request) = AnalyzeTelemetry(token, includeTraceContext: true, includePdfHeader: true); + ActivitySpanId parentSpanId; + using (var parentActivity = telemetry.Object.ActivitySource.StartActivity("TestParentActivity")) + { + Assert.NotNull(parentActivity); + parentSpanId = parentActivity.SpanId; + await request(); + } + await telemetry.WaitForServerActivity(); + + var activities = telemetry.CapturedActivities; + var activity = Assert.Single(activities, a => a.Kind == ActivityKind.Server); + Assert.True(activity.IsAllDataRequested); + Assert.True(activity.Recorded); + Assert.Equal("Microsoft.AspNetCore", activity.Source.Name); + Assert.NotNull(activity.ParentId); + Assert.Equal(parentSpanId, activity.ParentSpanId); + + await telemetry.Snapshot(activity, c => c.ScrubMember(Telemetry.Labels.UserPartyId)); + } } diff --git a/test/Altinn.App.Core.Tests/Internal/Pdf/PdfServiceTests.cs b/test/Altinn.App.Core.Tests/Internal/Pdf/PdfServiceTests.cs index aafcbc907..fbd689d81 100644 --- a/test/Altinn.App.Core.Tests/Internal/Pdf/PdfServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Pdf/PdfServiceTests.cs @@ -86,7 +86,9 @@ public async Task ValidRequest_ShouldReturnPdf() ); var httpClient = new HttpClient(delegatingHandler); + var logger = new Mock>(); var pdfGeneratorClient = new PdfGeneratorClient( + logger.Object, httpClient, _pdfGeneratorSettingsOptions, _platformSettingsOptions, @@ -114,7 +116,9 @@ public async Task ValidRequest_PdfGenerationFails_ShouldThrowException() ); var httpClient = new HttpClient(delegatingHandler); + var logger = new Mock>(); var pdfGeneratorClient = new PdfGeneratorClient( + logger.Object, httpClient, _pdfGeneratorSettingsOptions, _platformSettingsOptions,