OTel bridge enhancements (#2172)

* Optimise logic for `InferTransactionType`
* Simplify dictionary set-up
* Optimise access to attributes via Activity.TagObjects
* Update tests to latest client version
* Remove `ElasticSearchHttpNonTracer`

Author: stevejgordon

src/Elastic.Apm/OpenTelemetry/ElasticActivityListener.cs

@@ -22,8 +22,10 @@ namespace Elastic.Apm.OpenTelemetry
 {
 	public class ElasticActivityListener : IDisposable
 	{
-		private static readonly string[] ServerPortAttributeKeys = { SemanticConventions.ServerPort, SemanticConventions.NetPeerPort };
-		private static readonly string[] ServerAddressAttributeKeys = { SemanticConventions.ServerAddress, SemanticConventions.NetPeerName, SemanticConventions.NetPeerIp };
+		private static readonly string[] ServerPortAttributeKeys = new[] { SemanticConventions.ServerPort, SemanticConventions.NetPeerPort };
+		private static readonly string[] ServerAddressAttributeKeys = new[] { SemanticConventions.ServerAddress, SemanticConventions.NetPeerName, SemanticConventions.NetPeerIp };
+		private static readonly string[] HttpAttributeKeys = new[] { SemanticConventions.UrlFull, SemanticConventions.HttpUrl, SemanticConventions.HttpScheme };
+		private static readonly string[] HttpUrlAttributeKeys = new[] { SemanticConventions.UrlFull, SemanticConventions.HttpUrl };
 
 		private readonly ConditionalWeakTable<Activity, Span> _activeSpans = new();
 		private readonly ConditionalWeakTable<Activity, Transaction> _activeTransactions = new();
@@ -39,17 +41,16 @@ internal ElasticActivityListener(IApmAgent agent, HttpTraceConfiguration httpTra
 
 		internal void Start(Tracer tracerInternal)
 		{
-			_httpTraceConfiguration?.AddTracer(new ElasticSearchHttpNonTracer());
 			_tracer = tracerInternal;
 
-
 			Listener = new ActivityListener
 			{
 				ActivityStarted = ActivityStarted,
 				ActivityStopped = ActivityStopped,
 				ShouldListenTo = _ => true,
 				Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData
 			};
+
 			ActivitySource.AddActivityListener(Listener);
 		}
 
@@ -73,7 +74,6 @@ internal void Start(Tracer tracerInternal)
 				var timestamp = TimeUtils.ToTimestamp(activity.StartTimeUtc);
 				if (!CreateTransactionForActivity(activity, timestamp, spanLinks))
 					CreateSpanForActivity(activity, timestamp, spanLinks);
-
 			};
 
 		private bool CreateTransactionForActivity(Activity activity, long timestamp, List<SpanLink> spanLinks)
@@ -153,11 +153,8 @@ private void CreateSpanForActivity(Activity activity, long timestamp, List<SpanL
 					_activeTransactions.Remove(activity);
 					transaction.Duration = activity.Duration.TotalMilliseconds;
 
-					if (activity.TagObjects.Any())
-						transaction.Otel.Attributes = new Dictionary<string, object>();
-
-					foreach (var tag in activity.TagObjects)
-						transaction.Otel.Attributes.Add(tag.Key, tag.Value);
+						if (activity.TagObjects.Any())
+							transaction.Otel.Attributes = new Dictionary<string, object>(activity.TagObjects);
 
 					InferTransactionType(transaction, activity);
 
@@ -192,10 +189,7 @@ private static void UpdateSpan(Activity activity, Span span)
 			span.Duration = activity.Duration.TotalMilliseconds;
 
 			if (activity.TagObjects.Any())
-				span.Otel.Attributes = new Dictionary<string, object>();
-
-			foreach (var tag in activity.TagObjects)
-				span.Otel.Attributes.Add(tag.Key, tag.Value);
+				span.Otel.Attributes = new Dictionary<string, object>(activity.TagObjects);
 
 			InferSpanTypeAndSubType(span, activity);
 
@@ -218,17 +212,17 @@ private static void UpdateSpan(Activity activity, Span span)
 			span.End();
 		}
 
+		/// <summary>
+		/// Specifically exposed for benchmarking. This is not intended for any other purpose.
+		/// </summary>
 		internal static void UpdateSpanBenchmark(Activity activity, Span span) => UpdateSpan(activity, span);
 
 		private static void InferTransactionType(Transaction transaction, Activity activity)
 		{
-			var isRpc = activity.Tags.Any(n => n.Key == SemanticConventions.RpcSystem);
-			var isHttp = activity.Tags.Any(n => n.Key == SemanticConventions.HttpUrl || n.Key == SemanticConventions.UrlFull || n.Key == SemanticConventions.HttpScheme);
-			var isMessaging = activity.Tags.Any(n => n.Key == SemanticConventions.MessagingSystem);
-
-			if (activity.Kind == ActivityKind.Server && (isRpc || isHttp))
+			if (activity.Kind == ActivityKind.Server && (TryGetStringValue(activity, SemanticConventions.RpcSystem, out _)
+					|| TryGetStringValue(activity, HttpAttributeKeys, out _)))
 				transaction.Type = ApiConstants.TypeRequest;
-			else if (activity.Kind == ActivityKind.Consumer && isMessaging)
+			else if (activity.Kind == ActivityKind.Consumer && TryGetStringValue(activity, SemanticConventions.MessagingSystem, out _))
 				transaction.Type = ApiConstants.TypeMessaging;
 			else
 				transaction.Type = "unknown";
@@ -284,51 +278,45 @@ static string ToResourceName(string type, string name)
 			string serviceTargetName = null;
 			string resource = null;
 
-			var isDbSpan = TryGetStringValue(activity, SemanticConventions.DbSystem, out var dbSystem);
-
-			if (isDbSpan)
+			if (TryGetStringValue(activity, SemanticConventions.DbSystem, out var dbSystem))
 			{
 				span.Type = ApiConstants.TypeDb;
 				span.Subtype = dbSystem;
 				serviceTargetType = span.Subtype;
 				serviceTargetName = TryGetStringValue(activity, SemanticConventions.DbName, out var dbName) ? dbName : null;
 				resource = ToResourceName(span.Subtype, serviceTargetName);
 			}
-			else if (activity.Tags.Any(n => n.Key == SemanticConventions.MessagingSystem))
+			else if (TryGetStringValue(activity, SemanticConventions.MessagingSystem, out var messagingSystem))
 			{
 				span.Type = ApiConstants.TypeMessaging;
-				span.Subtype = activity.Tags.First(n => n.Key == SemanticConventions.MessagingSystem).Value;
+				span.Subtype = messagingSystem;
 				serviceTargetType = span.Subtype;
-				serviceTargetName = activity.Tags.FirstOrDefault(n => n.Key == SemanticConventions.MessagingDestination).Value;
+				serviceTargetName = TryGetStringValue(activity, SemanticConventions.MessagingDestination, out var messagingDestination) ? messagingDestination : null;
 				resource = ToResourceName(span.Subtype, serviceTargetName);
 			}
-			else if (activity.Tags.Any(n => n.Key == SemanticConventions.RpcSystem))
+			else if (TryGetStringValue(activity, SemanticConventions.RpcSystem, out var rpcSystem))
 			{
 				span.Type = ApiConstants.TypeExternal;
-				span.Subtype = activity.Tags.First(n => n.Key == SemanticConventions.RpcSystem).Value;
+				span.Subtype = rpcSystem;
 				serviceTargetType = span.Subtype;
 				serviceTargetName = !string.IsNullOrEmpty(netName)
 					? netName
-					: activity.Tags.FirstOrDefault(n => n.Key == SemanticConventions.RpcService).Value;
+					: TryGetStringValue(activity, SemanticConventions.RpcService, out var rpcService) ? rpcService : null;
 				resource = serviceTargetName ?? span.Subtype;
 			}
-			else if (activity.Tags.Any(n => n.Key == SemanticConventions.HttpUrl || n.Key == SemanticConventions.UrlFull || n.Key == SemanticConventions.HttpScheme))
+			else if (activity.TagObjects.Any(n => n.Key == SemanticConventions.HttpUrl || n.Key == SemanticConventions.UrlFull || n.Key == SemanticConventions.HttpScheme))
 			{
+				var hasHttpHost = TryGetStringValue(activity, SemanticConventions.HttpHost, out var httpHost);
+				var hasHttpScheme = TryGetStringValue(activity, SemanticConventions.HttpScheme, out var httpScheme);
 				span.Type = ApiConstants.TypeExternal;
 				span.Subtype = ApiConstants.SubtypeHttp;
 				serviceTargetType = span.Subtype;
-				if (activity.Tags.Any(n => n.Key == SemanticConventions.HttpHost) && activity.Tags.Any(n => n.Key == SemanticConventions.HttpScheme))
-				{
-					serviceTargetName = activity.Tags.FirstOrDefault(n => n.Key == SemanticConventions.HttpHost).Value + ":"
-						+ HttpPortFromScheme(activity.Tags.FirstOrDefault(n => n.Key == SemanticConventions.HttpScheme).Value);
-				}
-				else if (activity.Tags.Any(n => n.Key == SemanticConventions.HttpUrl))
-					serviceTargetName = ParseNetName(activity.Tags.FirstOrDefault(n => n.Key == SemanticConventions.HttpUrl).Value);
-				else if (activity.Tags.Any(n => n.Key == SemanticConventions.UrlFull))
-					serviceTargetName = ParseNetName(activity.Tags.FirstOrDefault(n => n.Key == SemanticConventions.UrlFull).Value);
+				if (hasHttpHost && hasHttpScheme)
+					serviceTargetName = $"{httpHost}:{HttpPortFromScheme(httpScheme)}";
+				else if (TryGetStringValue(activity, HttpUrlAttributeKeys, out var httpUrl))
+					serviceTargetName = ParseNetName(httpUrl);
 				else
 					serviceTargetName = netName;
-
 				resource = serviceTargetName;
 			}
 

src/Elastic.Apm/OpenTelemetry/ElasticSearchHttpNonTracer.cs

@@ -7,14 +7,13 @@
     </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Elastic.Clients.Elasticsearch" Version="8.0.1" />
+    <PackageReference Include="Elastic.Clients.Elasticsearch" Version="8.9.2" />
     <PackageReference Include="Testcontainers.Elasticsearch" Version="3.4.0" />
   </ItemGroup>
 
   <ItemGroup>
     <!--Only adding this for testing: make sure `ElasticsearchDiagnosticsSubscriber` does not cause any issue with the new client  -->
     <ProjectReference Include="$(SrcInstrumentations)\Elastic.Apm.Elasticsearch\Elastic.Apm.Elasticsearch.csproj" />
-
     <ProjectReference Include="$(SolutionRoot)\src\Elastic.Apm\Elastic.Apm.csproj" />
     <ProjectReference Include="$(SolutionRoot)\test\Elastic.Apm.Tests.Utilities\Elastic.Apm.Tests.Utilities.csproj" />
   </ItemGroup>