From f0639cbd4252a38ed4d7f95ec5d38225b14cb349 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 7 May 2025 10:59:44 +0200 Subject: [PATCH] Refactor OpenTelemetry error handling implementation (#6621) * Refactor OpenTelemetry error handling implementation Introduce WorkflowErrorSpanHandler interface and context for improved error span handling in workflows. Separate activity and workflow error handling using dedicated abstractions and context models. Update error handling logic in tracing middleware and adjust DI configuration accordingly. * Rename handler class and improve error tagging logic Renamed `FaultExceptionActivityErrorSpanHandler` to `FaultExceptionErrorSpanHandler` for consistency and clarity. Updated error attribute tagging to align with Datadog's well-known attributes. Adjusted incident selection logic to use the first incident instead of the last. * Align .gitignore file with consistent formatting Standardized comments in the .gitignore file by adding missing spaces and capitalizing as needed. Updated the `/docker/data/` entry to `/docker/azurite-data/` for clarity. * Fix incorrect selection of activity execution context Replaced `LastOrDefault` with `FirstOrDefault` to ensure the correct activity execution context is retrieved when handling errors. This change resolves potential inaccuracies in identifying the faulted activity node. * Exclude docker-compose-datadog.yml from solution file. --- .gitignore | 18 +++++----- src/apps/Elsa.Server.Web/Program.cs | 6 ++-- .../ActivityErrorSpanHandlerBase.cs | 11 +++++++ .../Abstractions/ErrorSpanHandlerBase.cs | 11 ------- .../WorkflowErrorSpanHandlerBase.cs | 11 +++++++ .../Contracts/IActivityErrorSpanHandler.cs | 10 ++++++ .../Contracts/IErrorSpanHandler.cs | 11 ------- .../Contracts/IWorkflowErrorSpanHandler.cs | 10 ++++++ .../Features/OpenTelemetryFeature.cs | 6 ++-- .../Handlers/DefaultErrorSpanHandler.cs | 18 ++++++---- .../FaultExceptionErrorSpanHandler.cs | 28 ++++++++++++---- ...metryTracingActivityExecutionMiddleware.cs | 4 +-- ...metryTracingWorkflowExecutionMiddleware.cs | 33 ++++++++++++++++--- ...Context.cs => ActivityErrorSpanContext.cs} | 2 +- .../Models/WorkflowErrorSpanContext.cs | 11 +++++++ 15 files changed, 134 insertions(+), 56 deletions(-) create mode 100644 src/modules/Elsa.OpenTelemetry/Abstractions/ActivityErrorSpanHandlerBase.cs delete mode 100644 src/modules/Elsa.OpenTelemetry/Abstractions/ErrorSpanHandlerBase.cs create mode 100644 src/modules/Elsa.OpenTelemetry/Abstractions/WorkflowErrorSpanHandlerBase.cs create mode 100644 src/modules/Elsa.OpenTelemetry/Contracts/IActivityErrorSpanHandler.cs delete mode 100644 src/modules/Elsa.OpenTelemetry/Contracts/IErrorSpanHandler.cs create mode 100644 src/modules/Elsa.OpenTelemetry/Contracts/IWorkflowErrorSpanHandler.cs rename src/modules/Elsa.OpenTelemetry/Models/{ErrorSpanContext.cs => ActivityErrorSpanContext.cs} (66%) create mode 100644 src/modules/Elsa.OpenTelemetry/Models/WorkflowErrorSpanContext.cs diff --git a/.gitignore b/.gitignore index 8a28551f4..68a650b2a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,7 @@ -#Ignore thumbnails created by Windows +# Ignore thumbnails created by Windows Thumbs.db -#Ignore files built by Visual Studio +# Ignore files built by Visual Studio *.obj *.exe *.pdb @@ -32,26 +32,26 @@ _ReSharper*/ # VS Code/Omnisharp crash dumps mono_crash.mem*.blob -#Nuget packages folder +# Nuget packages folder packages/ -#Ignore git-related files +# Ignore git-related files *.orig -#Rider +# Rider .idea .run -#wwwroot +# wwwroot wwwroot/ -#npm +# npm node_modules/ .babelrc .stylelintrc.yml .eslintrc.yml -#volatile +# volatile App_Data/ *.db *.db-journal @@ -78,5 +78,5 @@ unlist.sh /artifacts /docker/data/ - +/docker/azurite-data/ docker/docker-compose-datadog.yml diff --git a/src/apps/Elsa.Server.Web/Program.cs b/src/apps/Elsa.Server.Web/Program.cs index 63b49f2ff..c3e315e26 100644 --- a/src/apps/Elsa.Server.Web/Program.cs +++ b/src/apps/Elsa.Server.Web/Program.cs @@ -105,7 +105,7 @@ const bool useTenantsFromConfiguration = true; const bool useSecrets = false; const bool disableVariableWrappers = false; const bool disableVariableCopying = false; -const bool useManualOtelInstrumentation = false; +const bool useManualOtelInstrumentation = true; ObjectConverter.StrictMode = false; @@ -137,7 +137,7 @@ TypeAliasRegistry.RegisterAlias("OrderReceivedConsumerFactory", typeof(GenericCo if (useManualOtelInstrumentation) { services.AddOpenTelemetry() - .ConfigureResource(resource => resource.AddService("elsa-workflows", serviceVersion: "3.4.0").AddTelemetrySdk()) + .ConfigureResource(resource => resource.AddService("elsa-workflows", serviceVersion: "3.5.0").AddTelemetrySdk()) .WithTracing(tracing => { tracing @@ -536,7 +536,7 @@ services alterations.UseMassTransitDispatcher(); } }) - .UseOpenTelemetry() + .UseOpenTelemetry(otel => otel.UseNewRootActivityForRemoteParent = true) .UseWorkflowContexts(); if (useQuartz) diff --git a/src/modules/Elsa.OpenTelemetry/Abstractions/ActivityErrorSpanHandlerBase.cs b/src/modules/Elsa.OpenTelemetry/Abstractions/ActivityErrorSpanHandlerBase.cs new file mode 100644 index 000000000..517045897 --- /dev/null +++ b/src/modules/Elsa.OpenTelemetry/Abstractions/ActivityErrorSpanHandlerBase.cs @@ -0,0 +1,11 @@ +using Elsa.OpenTelemetry.Contracts; +using Elsa.OpenTelemetry.Models; + +namespace Elsa.OpenTelemetry.Abstractions; + +public abstract class ActivityErrorSpanHandlerBase : IActivityErrorSpanHandler +{ + public virtual float Order => 0; + public abstract bool CanHandle(ActivityErrorSpanContext context); + public abstract void Handle(ActivityErrorSpanContext context); +} \ No newline at end of file diff --git a/src/modules/Elsa.OpenTelemetry/Abstractions/ErrorSpanHandlerBase.cs b/src/modules/Elsa.OpenTelemetry/Abstractions/ErrorSpanHandlerBase.cs deleted file mode 100644 index 06e77bd4f..000000000 --- a/src/modules/Elsa.OpenTelemetry/Abstractions/ErrorSpanHandlerBase.cs +++ /dev/null @@ -1,11 +0,0 @@ -using Elsa.OpenTelemetry.Contracts; -using Elsa.OpenTelemetry.Models; - -namespace Elsa.OpenTelemetry.Abstractions; - -public abstract class ErrorSpanHandlerBase : IErrorSpanHandler -{ - public virtual float Order => 0; - public abstract bool CanHandle(ErrorSpanContext context); - public abstract void Handle(ErrorSpanContext context); -} \ No newline at end of file diff --git a/src/modules/Elsa.OpenTelemetry/Abstractions/WorkflowErrorSpanHandlerBase.cs b/src/modules/Elsa.OpenTelemetry/Abstractions/WorkflowErrorSpanHandlerBase.cs new file mode 100644 index 000000000..53c41c659 --- /dev/null +++ b/src/modules/Elsa.OpenTelemetry/Abstractions/WorkflowErrorSpanHandlerBase.cs @@ -0,0 +1,11 @@ +using Elsa.OpenTelemetry.Contracts; +using Elsa.OpenTelemetry.Models; + +namespace Elsa.OpenTelemetry.Abstractions; + +public abstract class WorkflowErrorSpanHandlerBase : IWorkflowErrorSpanHandler +{ + public virtual float Order => 0; + public abstract bool CanHandle(WorkflowErrorSpanContext context); + public abstract void Handle(WorkflowErrorSpanContext context); +} \ No newline at end of file diff --git a/src/modules/Elsa.OpenTelemetry/Contracts/IActivityErrorSpanHandler.cs b/src/modules/Elsa.OpenTelemetry/Contracts/IActivityErrorSpanHandler.cs new file mode 100644 index 000000000..0f637be42 --- /dev/null +++ b/src/modules/Elsa.OpenTelemetry/Contracts/IActivityErrorSpanHandler.cs @@ -0,0 +1,10 @@ +using Elsa.OpenTelemetry.Models; + +namespace Elsa.OpenTelemetry.Contracts; + +public interface IActivityErrorSpanHandler +{ + float Order { get; } + bool CanHandle(ActivityErrorSpanContext context); + void Handle(ActivityErrorSpanContext context); +} \ No newline at end of file diff --git a/src/modules/Elsa.OpenTelemetry/Contracts/IErrorSpanHandler.cs b/src/modules/Elsa.OpenTelemetry/Contracts/IErrorSpanHandler.cs deleted file mode 100644 index 70b16f645..000000000 --- a/src/modules/Elsa.OpenTelemetry/Contracts/IErrorSpanHandler.cs +++ /dev/null @@ -1,11 +0,0 @@ -using Elsa.OpenTelemetry.Models; - -namespace Elsa.OpenTelemetry.Contracts; - -public interface IErrorSpanHandler -{ - float Order { get; } - bool CanHandle(ErrorSpanContext context); - void Handle(ErrorSpanContext context); -} - diff --git a/src/modules/Elsa.OpenTelemetry/Contracts/IWorkflowErrorSpanHandler.cs b/src/modules/Elsa.OpenTelemetry/Contracts/IWorkflowErrorSpanHandler.cs new file mode 100644 index 000000000..f33265273 --- /dev/null +++ b/src/modules/Elsa.OpenTelemetry/Contracts/IWorkflowErrorSpanHandler.cs @@ -0,0 +1,10 @@ +using Elsa.OpenTelemetry.Models; + +namespace Elsa.OpenTelemetry.Contracts; + +public interface IWorkflowErrorSpanHandler +{ + float Order { get; } + bool CanHandle(WorkflowErrorSpanContext context); + void Handle(WorkflowErrorSpanContext context); +} \ No newline at end of file diff --git a/src/modules/Elsa.OpenTelemetry/Features/OpenTelemetryFeature.cs b/src/modules/Elsa.OpenTelemetry/Features/OpenTelemetryFeature.cs index 7b5542590..2e5d1ac3a 100644 --- a/src/modules/Elsa.OpenTelemetry/Features/OpenTelemetryFeature.cs +++ b/src/modules/Elsa.OpenTelemetry/Features/OpenTelemetryFeature.cs @@ -23,8 +23,10 @@ public class OpenTelemetryFeature(IModule module) : FeatureBase(module) public override void Configure() { Services - .AddScoped() - .AddScoped(); + .AddScoped() + .AddScoped() + .AddScoped() + .AddScoped(); Services.Configure(options => { diff --git a/src/modules/Elsa.OpenTelemetry/Handlers/DefaultErrorSpanHandler.cs b/src/modules/Elsa.OpenTelemetry/Handlers/DefaultErrorSpanHandler.cs index aac9fbae9..77c05e6d6 100644 --- a/src/modules/Elsa.OpenTelemetry/Handlers/DefaultErrorSpanHandler.cs +++ b/src/modules/Elsa.OpenTelemetry/Handlers/DefaultErrorSpanHandler.cs @@ -1,15 +1,21 @@ -using Elsa.OpenTelemetry.Abstractions; +using Elsa.OpenTelemetry.Contracts; using Elsa.OpenTelemetry.Models; namespace Elsa.OpenTelemetry.Handlers; -public class DefaultErrorSpanHandler : ErrorSpanHandlerBase +public class DefaultErrorSpanHandler : IActivityErrorSpanHandler, IWorkflowErrorSpanHandler { - public override float Order => 100000; - - public override bool CanHandle(ErrorSpanContext context) => context.Exception != null; + public float Order => 100000; - public override void Handle(ErrorSpanContext context) + public bool CanHandle(WorkflowErrorSpanContext context) => false; + + public void Handle(WorkflowErrorSpanContext context) + { + } + + public bool CanHandle(ActivityErrorSpanContext context) => context.Exception != null; + + public void Handle(ActivityErrorSpanContext context) { context.Span.AddException(context.Exception!); } diff --git a/src/modules/Elsa.OpenTelemetry/Handlers/FaultExceptionErrorSpanHandler.cs b/src/modules/Elsa.OpenTelemetry/Handlers/FaultExceptionErrorSpanHandler.cs index 9403e20e9..355e5d32b 100644 --- a/src/modules/Elsa.OpenTelemetry/Handlers/FaultExceptionErrorSpanHandler.cs +++ b/src/modules/Elsa.OpenTelemetry/Handlers/FaultExceptionErrorSpanHandler.cs @@ -1,23 +1,39 @@ -using Elsa.OpenTelemetry.Abstractions; +using Elsa.OpenTelemetry.Contracts; using Elsa.OpenTelemetry.Models; using Elsa.Workflows.Exceptions; namespace Elsa.OpenTelemetry.Handlers; -public class FaultExceptionErrorSpanHandler : ErrorSpanHandlerBase +public class FaultExceptionErrorSpanHandler : IActivityErrorSpanHandler, IWorkflowErrorSpanHandler { - public override bool CanHandle(ErrorSpanContext context) => context.Exception is FaultException; + public float Order => 0; - public override void Handle(ErrorSpanContext context) + public bool CanHandle(ActivityErrorSpanContext context) => context.Exception is FaultException; + public bool CanHandle(WorkflowErrorSpanContext context) => context.Exception is FaultException; + + public void Handle(ActivityErrorSpanContext context) { var faultException = (FaultException)context.Exception!; var span = context.Span; - var tags = new Dictionary + var tags = new Dictionary() { ["exception.code"] = faultException.Code, ["exception.category"] = faultException.Category, ["exception.type"] = faultException.Type }; - span.AddException(context.Exception, new(tags.ToArray())); + span.AddException(faultException, new(tags.ToArray())); + } + + public void Handle(WorkflowErrorSpanContext context) + { + var faultException = (FaultException)context.Exception!; + var span = context.Span; + + // The following two attributes are well-known by datadog. + span.SetTag("error.code", faultException.Code); + span.SetTag("error.category", faultException.Category); + + // Datadog will ignore unknown attributes, so we'll set them on a different object. + span.SetTag("error_details.type", faultException.Type); } } \ No newline at end of file diff --git a/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingActivityExecutionMiddleware.cs b/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingActivityExecutionMiddleware.cs index 6115f7e2d..617f4acb2 100644 --- a/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingActivityExecutionMiddleware.cs +++ b/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingActivityExecutionMiddleware.cs @@ -50,8 +50,8 @@ public class OpenTelemetryTracingActivityExecutionMiddleware(ActivityMiddlewareD span.AddEvent(new("faulted")); span.SetStatus(ActivityStatusCode.Error); - var errorSpanHandlerContext = new ErrorSpanContext(span, context.Exception); - var errorSpanHandler = context.GetServices() + var errorSpanHandlerContext = new ActivityErrorSpanContext(span, context.Exception); + var errorSpanHandler = context.GetServices() .OrderBy(x => x.Order) .FirstOrDefault(x => x.CanHandle(errorSpanHandlerContext)); diff --git a/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingWorkflowExecutionMiddleware.cs b/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingWorkflowExecutionMiddleware.cs index cbb1c583f..5d3996ea2 100644 --- a/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingWorkflowExecutionMiddleware.cs +++ b/src/modules/Elsa.OpenTelemetry/Middleware/OpenTelemetryTracingWorkflowExecutionMiddleware.cs @@ -1,6 +1,8 @@ using System.Diagnostics; using Elsa.Common; +using Elsa.OpenTelemetry.Contracts; using Elsa.OpenTelemetry.Helpers; +using Elsa.OpenTelemetry.Models; using Elsa.OpenTelemetry.Options; using Elsa.Workflows; using Elsa.Workflows.Models; @@ -56,7 +58,28 @@ public class OpenTelemetryTracingWorkflowExecutionMiddleware(WorkflowMiddlewareD if (context.SubStatus == WorkflowSubStatus.Faulted) { span.AddEvent(new("faulted")); - span.SetStatus(ActivityStatusCode.Error, "The workflow entered the Faulted state. See incidents for details."); + + var lastIncident = context.Incidents.FirstOrDefault(); + + if (lastIncident == null) + span.SetStatus(ActivityStatusCode.Error, "The workflow entered the Faulted state. See incidents for details."); + else + { + span.SetStatus(ActivityStatusCode.Error, lastIncident.Message); + + var activityExecutionContext = context.ActivityExecutionContexts.FirstOrDefault(x => x.Activity.NodeId == lastIncident.ActivityNodeId && x.Status == ActivityStatus.Faulted); + var exception = activityExecutionContext?.Exception; + + if (exception != null) + { + var errorSpanHandlerContext = new WorkflowErrorSpanContext(span, lastIncident, exception); + var errorSpanHandler = context.GetServices() + .OrderBy(x => x.Order) + .FirstOrDefault(x => x.CanHandle(errorSpanHandlerContext)); + + errorSpanHandler?.Handle(errorSpanHandlerContext); + } + } } else if (context.SubStatus == WorkflowSubStatus.Finished) { @@ -88,7 +111,7 @@ public class OpenTelemetryTracingWorkflowExecutionMiddleware(WorkflowMiddlewareD var startNewTraceOptionValue = context.Properties.TryGetValue("StartNewTrace", out var startNewTraceValue) && (bool)startNewTraceValue; var startNewTraceForRemoteParent = options.Value.UseNewRootActivityForRemoteParent && Activity.Current?.HasRemoteParent == true; var startNewTrace = startNewTraceOptionValue || startNewTraceForRemoteParent; - + ActivityContext contextToUse; ActivityContext? linkedTraceContext = null; @@ -108,10 +131,10 @@ public class OpenTelemetryTracingWorkflowExecutionMiddleware(WorkflowMiddlewareD } var span = ElsaOpenTelemetry.ActivitySource.StartActivity($"execute workflow {workflowName}", ActivityKind.Server, contextToUse); - + if (span != null && linkedTraceContext != null) - span.AddLink(new (linkedTraceContext.Value)); - + span.AddLink(new(linkedTraceContext.Value)); + return span; } diff --git a/src/modules/Elsa.OpenTelemetry/Models/ErrorSpanContext.cs b/src/modules/Elsa.OpenTelemetry/Models/ActivityErrorSpanContext.cs similarity index 66% rename from src/modules/Elsa.OpenTelemetry/Models/ErrorSpanContext.cs rename to src/modules/Elsa.OpenTelemetry/Models/ActivityErrorSpanContext.cs index a364b1b68..0eab206db 100644 --- a/src/modules/Elsa.OpenTelemetry/Models/ErrorSpanContext.cs +++ b/src/modules/Elsa.OpenTelemetry/Models/ActivityErrorSpanContext.cs @@ -2,7 +2,7 @@ using System.Diagnostics; namespace Elsa.OpenTelemetry.Models; -public class ErrorSpanContext(Activity span, Exception? exception) +public class ActivityErrorSpanContext(Activity span, Exception? exception) { public Activity Span => span; public Exception? Exception => exception; diff --git a/src/modules/Elsa.OpenTelemetry/Models/WorkflowErrorSpanContext.cs b/src/modules/Elsa.OpenTelemetry/Models/WorkflowErrorSpanContext.cs new file mode 100644 index 000000000..b5377d6ee --- /dev/null +++ b/src/modules/Elsa.OpenTelemetry/Models/WorkflowErrorSpanContext.cs @@ -0,0 +1,11 @@ +using System.Diagnostics; +using Elsa.Workflows.Models; + +namespace Elsa.OpenTelemetry.Models; + +public class WorkflowErrorSpanContext(Activity span, ActivityIncident incident, Exception? exception) +{ + public Activity Span => span; + public ActivityIncident Incident => incident; + public Exception? Exception { get; } = exception; +} \ No newline at end of file