Fix Client Diagnostics following upgrade to DiagnosticSource 5.0.0 (#5326) (#5327)

Relates: elastic/apm-agent-dotnet#1094

This commit fixes a rather pernicious bug with Diagnostics from the clients,
following the upgrade to System.Diagnostics.DiagnosticSource 5.0.0.

`Diagnostic` and `Diagnostic<TState, TStateEnd>` derived from
System.Diagnostics.Activity, implementing IDisposable to make them
nicer to work with. In System.Diagnostics.DiagnosticSource 5.0.0, Activity now
implements IDisposable, so the impl was updated to override `Dispose(bool)`.

A problem arises from now deriving from Activity in that for an activity that is not finished,
`Dispose()` calls `Stop()` which in turn will notify the ActivitySource that it has stopped
**and** sets `Activity.Current` to the parent Activity (null if there's no parent). Now,
when calling into overridden `Dispose(bool)`, the DiagnosticSource is notified that
the `Activity` has stopped, allowing for listeners to take action. If a listener is getting
the Activity through `Activity.Current` as Elastic APM's integration does however,
`Activity.Current` no longer represents the Activity that has stopped, but its parent, with
no nice way to reference the stopped Activity.

This commit removes deriving from `Activity` and introduces a private field
in which to hold an Activity that is started, and ended on Dispose. Starting
and stopping the activity sets the start and end times, respectively.

Co-authored-by: Russ Cam <[email protected]>

Author: github-actions[bot]

src/Elasticsearch.Net/Diagnostics/Diagnostic.cs

@@ -5,10 +5,10 @@
 using System;
 using System.Diagnostics;
 
-namespace Elasticsearch.Net.Diagnostics 
+namespace Elasticsearch.Net.Diagnostics
 {
 	/// <summary>
-	/// Internal subclass of <see cref="Activity"/> that implements <see cref="IDisposable"/> to
+	/// Diagnostic that creates, starts and stops <see cref="Activity"/> that implements <see cref="IDisposable"/> to
 	/// make it easier to use.
 	/// </summary>
 	internal class Diagnostic<TState> : Diagnostic<TState, TState>
@@ -17,48 +17,40 @@ public Diagnostic(string operationName, DiagnosticSource source, TState state)
 			: base(operationName, source, state) =>
 			EndState = state;
 	}
-	
-	internal class Diagnostic<TState, TStateEnd> : Activity
+
+	internal class Diagnostic<TState, TStateEnd> : IDisposable
 	{
 		public static Diagnostic<TState, TStateEnd> Default { get; } = new();
 
 		private readonly DiagnosticSource _source;
-		private TStateEnd _endState;
 		private readonly bool _default;
-		private bool _disposed;
+		private readonly Activity _activity;
+		private TStateEnd _endState;
 
-		private Diagnostic() : base("__NOOP__") => _default = true;
+		private Diagnostic() => _default = true;
 
-		public Diagnostic(string operationName, DiagnosticSource source, TState state) : base(operationName)
+		public Diagnostic(string operationName, DiagnosticSource source, TState state)
 		{
 			_source = source;
-			_source.StartActivity(SetStartTime(DateTime.UtcNow), state);
+			_activity = new Activity(operationName);
+			_source.StartActivity(_activity, state);
 		}
 
 		public TStateEnd EndState
 		{
 			get => _endState;
 			internal set
 			{
-				//do not store state on default instance 
+				//do not store state on default instance
 				if (_default) return;
-				_endState =  value;	
+				_endState =  value;
 			}
 		}
-		
-		protected override void Dispose(bool disposing)
-		{
-			if (_disposed) return;
 
-			if (disposing)
-			{
-				//_source can be null if Default instance
-				_source?.StopActivity(SetEndTime(DateTime.UtcNow), EndState);
-			}
-
-			_disposed = true;
-
-			base.Dispose(disposing);
+		public void Dispose()
+		{
+			_source?.StopActivity(_activity, EndState);
+			_activity?.Dispose();
 		}
 	}
 }