Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@

<!-- NuGet -->
<PackageId>DSoftStudio.Mediator.OpenTelemetry</PackageId>
<Version>1.1.0-rc.3</Version>
<!-- 1.1.0-rc.4 — notification per-handler spans resolve the concrete subscriber through
IPipelineHandlerTypeAccessor, so a live-profiler wrapper no longer leaks its mangled type into
mediator.handler.type (ADR-0034 §3.6). Next after the real published rc.3. NOTE: stale unpublished
rc.4 copies must be purged from ~/.nuget/packages before restore, or the cache serves the old build
(same-version cache trap). -->
<Version>1.1.0-rc.4</Version>

<Authors>DSoftStudio</Authors>
<Company>DSoftStudio</Company>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
private static readonly ActivitySource Source = MediatorInstrumentation.ActivitySource;
private static readonly ConcurrentDictionary<Type, string> HandlerSpanNames = new();

public async Task Publish<TNotification>(

Check warning on line 19 in src/DSoftStudio.Mediator.OpenTelemetry/InstrumentedNotificationPublisher.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed.
IEnumerable<INotificationHandler<TNotification>> handlers,
TNotification notification,
CancellationToken cancellationToken)
Expand Down Expand Up @@ -47,8 +47,8 @@

if (parentActivity is { IsAllDataRequested: true })
{
parentActivity.SetTag("mediator.request.type", MediatorNotificationMetadata<TNotification>.RequestType);

Check warning on line 50 in src/DSoftStudio.Mediator.OpenTelemetry/InstrumentedNotificationPublisher.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Define a constant instead of using this literal 'mediator.request.type' 4 times.
parentActivity.SetTag("mediator.request.kind", MediatorNotificationMetadata<TNotification>.RequestKind);

Check warning on line 51 in src/DSoftStudio.Mediator.OpenTelemetry/InstrumentedNotificationPublisher.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Define a constant instead of using this literal 'mediator.request.kind' 4 times.

options.EnrichActivity?.Invoke(parentActivity, notification);
}
Expand Down Expand Up @@ -123,8 +123,16 @@

public async Task Handle(TNotification notification, CancellationToken cancellationToken)
{
// Resolve the CONCRETE handler type through any transparent decorator. When the live-profiler
// is also active it wraps this handler in a per-handler timing decorator that exposes the real
// subscriber via IPipelineHandlerTypeAccessor — the SAME seam the request path
// (MediatorDispatchTracingObserver) and stream path (MediatorStreamTracingBehavior) use. Reading
// it here keeps the span name + mediator.handler.type pointing at the real handler, never the
// profiler's compiler-mangled wrapper (which would collapse every subscriber to one type in the
// IDE fan-out and break the per-handler join).
var handlerType = ResolveHandlerType(inner);
var spanName = HandlerSpanNames.GetOrAdd(
inner.GetType(),
handlerType,
static type => $"{type.Name} handle");

using var activity = Source.StartActivity(spanName, ActivityKind.Internal);
Expand All @@ -132,14 +140,14 @@
// ADR-0047 P3 — tag the per-handler span so an imported trace can recognize it as a mediator
// NOTIFICATION HANDLER (not an unattributed Internal span) and map it to its handler source:
// • request.kind/type identify the published notification (so the importer pairs this child span
// with its NotificationPublished parent — the flame's nested-handler breakdown);

Check warning on line 143 in src/DSoftStudio.Mediator.OpenTelemetry/InstrumentedNotificationPublisher.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Remove this commented out code.
// • handler.type is the concrete subscriber, so the row jumps to the handler's own declaration
// and any HTTP/DB child span renders as a dependency UNDER this handler (ADR-0049).
if (activity is { IsAllDataRequested: true })
{
activity.SetTag("mediator.request.kind", MediatorNotificationMetadata<TNotification>.RequestKind);
activity.SetTag("mediator.request.type", MediatorNotificationMetadata<TNotification>.RequestType);
activity.SetTag("mediator.handler.type", inner.GetType().FullName);
activity.SetTag("mediator.handler.type", handlerType.FullName);
}

try
Expand All @@ -158,5 +166,12 @@
throw;
}
}

// Mirrors MediatorDispatchTracingObserver.ResolveHandlerType / MediatorStreamTracingBehavior:
// a transparent decorator (e.g. the live-profiler's per-handler wrapper) exposes the real handler
// through IPipelineHandlerTypeAccessor; a terminal handler does not implement it and reports its own
// runtime type.
private static Type ResolveHandlerType(INotificationHandler<TNotification> handler)
=> handler is IPipelineHandlerTypeAccessor accessor ? accessor.HandlerType : handler.GetType();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,46 @@ public async Task Handler_child_span_names_use_handler_type_name()
childNames.ShouldContain("TestNotificationHandler2 handle");
}

[Fact]
public async Task Handler_span_unwraps_transparent_wrapper_to_real_handler_type()
{
// Repro for the live-profiler + bridge coexistence bug: when the profiler wraps each notification
// handler in a per-handler timing decorator (TimedNotificationHandler<T>, which implements
// IPipelineHandlerTypeAccessor), the bridge must read the REAL handler type via that seam — NOT the
// decorator's compiler-mangled type — so the notification fan-out shows the concrete subscriber
// (SendConfirmationEmail/UpdateInventory), not <...>__TimedNotificationHandler.
using var collector = new ActivityCollector();
var options = new MediatorInstrumentationOptions();
var inner = new SequentialNotificationPublisher();
var publisher = new InstrumentedNotificationPublisher(inner, options, _metrics.Metrics);

var handlers = new INotificationHandler<TestNotification>[]
{
new TracingTransparentNotificationHandler<TestNotification>(new TestNotificationHandler1()),
new TracingTransparentNotificationHandler<TestNotification>(new TestNotificationHandler2()) // file-scoped wrapper below
};

await publisher.Publish(handlers, new TestNotification("hi"), TestContext.Current.CancellationToken);

var childSpans = collector.Activities
.Where(a => a.DisplayName.EndsWith(" handle"))
.ToList();
childSpans.Count.ShouldBe(2);

// Span NAME must use the real handler's type name, not the wrapper's mangled name.
var childNames = childSpans.Select(a => a.DisplayName).ToList();
childNames.ShouldContain("TestNotificationHandler1 handle");
childNames.ShouldContain("TestNotificationHandler2 handle");

// mediator.handler.type must be the REAL handler's FullName (what the IDE fan-out joins on).
var handlerTypes = childSpans
.Select(a => (string)a.GetTagItem("mediator.handler.type")!)
.ToList();
handlerTypes.ShouldContain(typeof(TestNotificationHandler1).FullName!);
handlerTypes.ShouldContain(typeof(TestNotificationHandler2).FullName!);
handlerTypes.ShouldNotContain(t => t.Contains("TracingTransparentNotificationHandler", StringComparison.Ordinal));
}

[Fact]
public async Task Error_in_handler_sets_error_status_on_parent_and_child()
{
Expand Down Expand Up @@ -224,3 +264,25 @@ public async Task EnrichActivity_callback_on_notification()
parentSpan.GetTagItem("custom.value")!.ShouldBe("enriched");
}
}

/// <summary>
/// A per-handler notification decorator that is TRANSPARENT to tracing: it wraps an inner handler
/// (for timing/profiling) yet exposes the real handler type via <see cref="IPipelineHandlerTypeAccessor"/>,
/// exactly like the Enterprise live-profiler's generated per-handler wrapper (TimedNotificationHandler&lt;T&gt;).
/// <para>
/// Declared <c>file</c> so the OSS DependencyInjectionGenerator skips it during handler discovery — an
/// open-generic <c>INotificationHandler&lt;TNotification&gt;</c> shape would otherwise be registered with an
/// unbound type parameter. This is the SAME reason the real profiler wrapper is file-scoped.
/// </para>
/// </summary>
file sealed class TracingTransparentNotificationHandler<TNotification>(INotificationHandler<TNotification> inner)
: INotificationHandler<TNotification>, IPipelineHandlerTypeAccessor
where TNotification : INotification
{
// Same recursive walk to the terminal handler as BehaviorHandlerAdapter / StreamPipelineChainHandler.
public Type HandlerType
=> inner is IPipelineHandlerTypeAccessor a ? a.HandlerType : inner.GetType();

public Task Handle(TNotification notification, CancellationToken cancellationToken)
=> inner.Handle(notification, cancellationToken);
}
Loading