summary: Add new span attributes to more closely match OTel spec. (#1976

)

feat: Add new span attributes to more closely match OTel spec. This includes server.address and server.port for database calls, and thread.id where appropriate.

src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs

@@ -43,7 +43,11 @@ public interface IAttributeDefinitions
         AttributeDefinition<float, double> DatabaseDuration { get; }
         AttributeDefinition<string, string> DbCollection { get; }
         AttributeDefinition<string, string> DbInstance { get; }
+        AttributeDefinition<string, string> DbOperation { get; }
+        AttributeDefinition<string, string> DbServerAddress { get; }
+        AttributeDefinition<long, long> DbServerPort { get; }
         AttributeDefinition<string, string> DbStatement { get; }
+        AttributeDefinition<string, string> DbSystem { get; }
         AttributeDefinition<string, string> DistributedTraceId { get; }
         AttributeDefinition<TimeSpan, double> Duration { get; }
         AttributeDefinition<bool, bool> IsErrorExpected { get; }
@@ -91,6 +95,8 @@ public interface IAttributeDefinitions
         AttributeDefinition<string, string> RequestUri { get; }
         AttributeDefinition<int?, string> ResponseStatus { get; }
         AttributeDefinition<bool, bool> Sampled { get; }
+        AttributeDefinition<string, string> ServerAddress { get; }
+        AttributeDefinition<long, long> ServerPort { get; }
         AttributeDefinition<SpanCategory, string> SpanCategory { get; }
         AttributeDefinition<string, string> SpanErrorClass { get; }
         AttributeDefinition<string, string> SpanErrorMessage { get; }
@@ -102,6 +108,7 @@ public interface IAttributeDefinitions
         AttributeDefinition<string, string> SyntheticsMonitorIdForTraces { get; }
         AttributeDefinition<string, string> SyntheticsResourceId { get; }
         AttributeDefinition<string, string> SyntheticsResourceIdForTraces { get; }
+        AttributeDefinition<long, long> ThreadId { get; }
         AttributeDefinition<DateTime, long> Timestamp { get; }
         AttributeDefinition<DateTime, long> TimestampForError { get; }
         AttributeDefinition<TimeSpan, double> TotalTime { get; }
@@ -476,6 +483,12 @@ public AttributeDefinitions(IAttributeFilter attribFilter)
                 .AppliesTo(AttributeDestinations.SpanEvent)
                 .Build(_attribFilter));
 
+        private AttributeDefinition<long, long> _threadId;
+        public AttributeDefinition<long, long> ThreadId => _threadId ?? (_threadId =
+            AttributeDefinitionBuilder.CreateLong("thread.id", AttributeClassification.Intrinsics)
+                .AppliesTo(AttributeDestinations.SpanEvent)
+                .Build(_attribFilter));
+
         private AttributeDefinition<string, string> _component;
         public AttributeDefinition<string, string> Component => _component ?? (_component =
             AttributeDefinitionBuilder.CreateString("component", AttributeClassification.Intrinsics)
@@ -513,6 +526,18 @@ public AttributeDefinitions(IAttributeFilter attribFilter)
                 .AppliesTo(AttributeDestinations.SpanEvent)
                 .Build(_attribFilter));
 
+        private AttributeDefinition<string, string> _dbSystem;
+        public AttributeDefinition<string, string> DbSystem => _dbSystem ?? (_dbSystem =
+            AttributeDefinitionBuilder.CreateString("db.system", AttributeClassification.AgentAttributes)
+                .AppliesTo(AttributeDestinations.SpanEvent)
+                .Build(_attribFilter));
+
+        private AttributeDefinition<string, string> _dbOperation;
+        public AttributeDefinition<string, string> DbOperation => _dbOperation ?? (_dbOperation =
+            AttributeDefinitionBuilder.CreateString("db.operation", AttributeClassification.AgentAttributes)
+                .AppliesTo(AttributeDestinations.SpanEvent)
+                .Build(_attribFilter));
+
         private AttributeDefinition<string, string> _dbCollection;
         public AttributeDefinition<string, string> DbCollection => _dbCollection ?? (_dbCollection =
             AttributeDefinitionBuilder.CreateString("db.collection", AttributeClassification.AgentAttributes)
@@ -546,7 +571,31 @@ public AttributeDefinitions(IAttributeFilter attribFilter)
 
         private AttributeDefinition<string, string> _httpMethod;
         public AttributeDefinition<string, string> HttpMethod => _httpMethod ?? (_httpMethod =
-            AttributeDefinitionBuilder.CreateString("http.method", AttributeClassification.AgentAttributes)
+            AttributeDefinitionBuilder.CreateString("http.request.method", AttributeClassification.AgentAttributes)
+                .AppliesTo(AttributeDestinations.SpanEvent)
+                .Build(_attribFilter));
+
+        private AttributeDefinition<string, string> _serverAddress;
+        public AttributeDefinition<string, string> ServerAddress => _serverAddress ?? (_serverAddress =
+            AttributeDefinitionBuilder.CreateString("server.address", AttributeClassification.Intrinsics)
+                .AppliesTo(AttributeDestinations.SpanEvent)
+                .Build(_attribFilter));
+
+        private AttributeDefinition<long, long> _serverPort;
+        public AttributeDefinition<long, long> ServerPort => _serverPort ?? (_serverPort =
+            AttributeDefinitionBuilder.CreateLong("server.port", AttributeClassification.Intrinsics)
+                .AppliesTo(AttributeDestinations.SpanEvent)
+                .Build(_attribFilter));
+
+        private AttributeDefinition<string, string> _dbServerAddress;
+        public AttributeDefinition<string, string> DbServerAddress => _dbServerAddress ?? (_dbServerAddress =
+            AttributeDefinitionBuilder.CreateString("server.address", AttributeClassification.AgentAttributes)
+                .AppliesTo(AttributeDestinations.SpanEvent)
+                .Build(_attribFilter));
+
+        private AttributeDefinition<long, long> _dbServerPort;
+        public AttributeDefinition<long, long> DbServerPort => _dbServerPort ?? (_dbServerPort =
+            AttributeDefinitionBuilder.CreateLong("server.port", AttributeClassification.AgentAttributes)
                 .AppliesTo(AttributeDestinations.SpanEvent)
                 .Build(_attribFilter));
 

src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs

@@ -23,15 +23,18 @@ namespace NewRelic.Agent.Core.Segments
 {
     public class DatastoreSegmentData : AbstractSegmentData, IDatastoreSegmentData
     {
-        private readonly static ConnectionInfo EmptyConnectionInfo = new ConnectionInfo(null, null, null);
+        private readonly static ConnectionInfo EmptyConnectionInfo = new ConnectionInfo(null, null, null, null);
 
         public override SpanCategory SpanCategory => SpanCategory.Datastore;
 
         public string Operation => _parsedSqlStatement.Operation;
         public DatastoreVendor DatastoreVendorName => _parsedSqlStatement.DatastoreVendor;
         public string Model => _parsedSqlStatement.Model;
         public string CommandText { get; set; }
+        public string Vendor => _connectionInfo.Vendor;
         public string Host => _connectionInfo.Host;
+        public int? Port => _connectionInfo.Port;
+        public string PathOrId => _connectionInfo.PathOrId;
         public string PortPathOrId => _connectionInfo.PortPathOrId;
         public string DatabaseName => _connectionInfo.DatabaseName;
         public Func<object> GetExplainPlanResources { get; set; }
@@ -219,10 +222,18 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection
                 AttribDefs.DbCollection.TrySetValue(attribVals, _parsedSqlStatement.Model);
             }
 
+            AttribDefs.DbSystem.TrySetValue(attribVals, Vendor);
             AttribDefs.DbInstance.TrySetValue(attribVals, DatabaseName);
+            AttribDefs.DbOperation.TrySetValue(attribVals, Operation);
             AttribDefs.PeerAddress.TrySetValue(attribVals, $"{Host}:{PortPathOrId}");
-            AttribDefs.PeerHostname.TrySetValue(attribVals, Host);
             AttribDefs.SpanKind.TrySetDefault(attribVals);
+            // peer.hostname and server.address must match
+            AttribDefs.PeerHostname.TrySetValue(attribVals, Host);
+            AttribDefs.DbServerAddress.TrySetValue(attribVals, Host);
+            if (Port != null)
+            {
+                AttribDefs.DbServerPort.TrySetValue(attribVals, Port.Value);
+            }
         }
 
         public void SetConnectionInfo(ConnectionInfo connInfo)

src/Agent/NewRelic/Agent/Core/Segments/ExternalSegmentData.cs

@@ -93,6 +93,8 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection
             AttribDefs.Component.TrySetValue(attribVals, _segmentState.TypeName);
             AttribDefs.SpanKind.TrySetDefault(attribVals);
             AttribDefs.HttpStatusCode.TrySetValue(attribVals, _httpStatusCode);   //Attrib handles null
+            AttribDefs.ServerAddress.TrySetValue(attribVals, Uri.Host);
+            AttribDefs.ServerPort.TrySetValue(attribVals, Uri.Port);
         }
 
         public override string GetTransactionTraceName()

src/Agent/NewRelic/Agent/Core/Segments/Segment.cs

@@ -53,6 +53,7 @@ public Segment(ITransactionSegmentState transactionSegmentState, MethodCallData
             Data.AttachSegmentDataState(this);
             Combinable = false;
             IsLeaf = false;
+            IsAsync = methodCallData.IsAsync;
         }
 
         /// <summary>
@@ -75,6 +76,7 @@ public Segment(ITransactionSegmentState transactionSegmentState, MethodCallData
             Data.AttachSegmentDataState(this);
             Combinable = false;
             IsLeaf = true;
+            IsAsync = methodCallData.IsAsync;
         }
 
         /// <summary>
@@ -105,6 +107,7 @@ public Segment(TimeSpan relativeStartTime, TimeSpan? duration, Segment segment,
             }
 
             SpanId = segment.SpanId;
+            IsAsync = segment.IsAsync;
         }
 
         public bool IsDone
@@ -229,6 +232,8 @@ public void RemoveSegmentFromCallStack()
         /// </summary>
         public int ThreadId { get; private set; }
 
+        public bool IsAsync { get; private set; }
+
         // used to set the ["unfinished"] parameter, not for real-time state of segment
         public bool Unfinished { get; private set; }
 
@@ -335,6 +340,11 @@ public SpanAttributeValueCollection GetAttributeValues()
                 AttribDefs.CodeFunction.TrySetValue(attribValues, codeFunction);
             }
 
+            if (!IsAsync)
+            {
+                AttribDefs.ThreadId.TrySetValue(attribValues, ThreadId);
+            }
+
             Data.SetSpanTypeSpecificAttributes(attribValues);
 
             return attribValues;
@@ -441,5 +451,6 @@ public string GetCategory()
         {
             return EnumNameCache<SpanCategory>.GetName(Data.SpanCategory);
         }
+
     }
 }

src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs

@@ -332,13 +332,13 @@ private static MethodCallData GetMethodCallData(MethodCall methodCall)
             var typeName = methodCall.Method.Type.FullName ?? "[unknown]";
             var methodName = methodCall.Method.MethodName;
             var invocationTargetHashCode = RuntimeHelpers.GetHashCode(methodCall.InvocationTarget);
-            return new MethodCallData(typeName, methodName, invocationTargetHashCode);
+            return new MethodCallData(typeName, methodName, invocationTargetHashCode, methodCall.IsAsync);
         }
 
         // Used for StackExchange.Redis since we will not be instrumenting any methods when creating the many DataStore segments
         private static MethodCallData GetMethodCallData(string typeName, string methodName, int invocationTargetHashCode)
         {
-            return new MethodCallData(typeName, methodName, invocationTargetHashCode);
+            return new MethodCallData(typeName, methodName, invocationTargetHashCode, true); // assume async
         }
 
         private static MetricNames.MessageBrokerDestinationType AgentWrapperApiEnumToMetricNamesEnum(

src/Agent/NewRelic/Agent/Core/Wrapper/AgentWrapperApi/Data/MethodCallData.cs

@@ -8,12 +8,14 @@ public class MethodCallData
         public readonly string TypeName;
         public readonly string MethodName;
         public readonly int InvocationTargetHashCode;
+        public readonly bool IsAsync;
 
-        public MethodCallData(string typeName, string methodName, int invocationTargetHashCode)
+        public MethodCallData(string typeName, string methodName, int invocationTargetHashCode, bool isAsync = false)
         {
             TypeName = typeName;
             MethodName = methodName;
             InvocationTargetHashCode = invocationTargetHashCode;
+            IsAsync = isAsync;
         }
 
         public override string ToString()
@@ -27,6 +29,7 @@ public override int GetHashCode()
             hash = hash * 23 + TypeName.GetHashCode();
             hash = hash * 23 + MethodName.GetHashCode();
             hash = hash * 23 + InvocationTargetHashCode;
+            hash = hash * 29 + IsAsync.GetHashCode();
             return hash;
         }
 
@@ -37,7 +40,8 @@ public override bool Equals(object obj)
                 var other = (MethodCallData)obj;
                 return InvocationTargetHashCode == other.InvocationTargetHashCode &&
                     MethodName.Equals(other.MethodName) &&
-                    TypeName.Equals(other.TypeName);
+                    TypeName.Equals(other.TypeName) &&
+                    IsAsync == other.IsAsync;
             }
             return false;
         }

src/Agent/NewRelic/Agent/Core/Wrapper/WrapperService.cs

@@ -142,7 +142,7 @@ public WrapperService(IConfigurationService configurationService, IWrapperMap wr
                 }
             }
 
-            var methodCall = new MethodCall(instrumentedMethodInfo.Method, invocationTarget, methodArguments);
+            var methodCall = new MethodCall(instrumentedMethodInfo.Method, invocationTarget, methodArguments, instrumentedMethodInfo.IsAsync);
             var instrumentedMethodCall = new InstrumentedMethodCall(methodCall, instrumentedMethodInfo);
 
             // if the wrapper throws an exception when executing the pre-method code, make sure the wrapper isn't called again in the future

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/IConnectionInfo.cs

@@ -5,10 +5,25 @@ namespace NewRelic.Agent.Extensions.Parsing
 {
     public class ConnectionInfo
     {
-        public ConnectionInfo(string host, string portPathOrId, string databaseName, string instanceName = null)
+        public ConnectionInfo(string vendor, string host, int port, string databaseName, string instanceName = null)
         {
+            Vendor = vendor;
             Host = ValueOrUnknown(host);
-            PortPathOrId = ValueOrUnknown(portPathOrId);
+            if (port >= 0)
+            {
+                Port = port;
+            }
+            PathOrId = ValueOrUnknown(string.Empty);
+            DatabaseName = ValueOrUnknown(databaseName);
+            InstanceName = instanceName;
+        }
+
+        public ConnectionInfo(string vendor, string host, string pathOrId, string databaseName, string instanceName = null)
+        {
+            Vendor = vendor;
+            Host = ValueOrUnknown(host);
+            Port = null;
+            PathOrId = ValueOrUnknown(pathOrId);
             DatabaseName = ValueOrUnknown(databaseName);
             InstanceName = instanceName;
         }
@@ -18,8 +33,11 @@ private static string ValueOrUnknown(string value)
             return string.IsNullOrEmpty(value) ? "unknown" : value;
         }
 
+        public string Vendor { get; private set; }
         public string Host { get; private set; }
-        public string PortPathOrId { get; private set; }
+        public string PortPathOrId { get => (Port != null) ? Port.ToString() : PathOrId; }
+        public int? Port { get; private set; } = null;
+        public string PathOrId { get; private set; } = string.Empty;
         public string DatabaseName { get; private set; }
         public string InstanceName { get; private set; }
     }

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Providers/Wrapper/Constants.cs

@@ -82,6 +82,24 @@ public enum DatastoreVendor
         Other
     }
 
+    public static class DatastoreVendorExtensions
+    {
+        // Convert our internal enum to the matching OTel "known" name for a database provider
+        public static string ToKnownName(this DatastoreVendor vendor)
+        {
+            switch (vendor)
+            {
+                case DatastoreVendor.Other:
+                    return "other_sql";
+                case DatastoreVendor.IBMDB2:
+                    return "db2";
+                // The others match our enum name
+                default:
+                    return EnumNameCache<DatastoreVendor>.GetNameToLower(vendor);
+            }
+        }
+    }
+
     public static class EnumNameCache<TEnum> // c# 7.3: where TEnum : System.Enum	
     {
         private static readonly ConcurrentDictionary<TEnum, string> Cache = new ConcurrentDictionary<TEnum, string>();

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Providers/Wrapper/MethodCall.cs

@@ -11,12 +11,14 @@ public class MethodCall
         public readonly Method Method;
         public readonly object InvocationTarget;
         public readonly object[] MethodArguments;
+        public readonly bool IsAsync;
 
-        public MethodCall(Method method, object invocationTarget, object[] methodArguments)
+        public MethodCall(Method method, object invocationTarget, object[] methodArguments, bool isAsync)
         {
             Method = method;
             InvocationTarget = invocationTarget;
             MethodArguments = methodArguments ?? new object[0];
+            IsAsync = isAsync;
         }
     }
 }

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AspNetCore/WrapPipelineMiddleware.cs

@@ -148,7 +148,7 @@ private ISegment SetupSegment(ITransaction transaction, HttpContext context)
         {
             // Seems like it would be cool to not require all of this for a segment??? 
             var method = new Method(typeof(WrapPipelineMiddleware), nameof(Invoke), nameof(context));
-            var methodCall = new MethodCall(method, this, new object[] { context });
+            var methodCall = new MethodCall(method, this, new object[] { context }, true);
 
             var segment = transaction.StartTransactionSegment(methodCall, "Middleware Pipeline");
             return segment;

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/CosmosDb/ExecuteItemQueryAsyncWrapper.cs

@@ -78,7 +78,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
             var segment = transaction.StartDatastoreSegment(
                 instrumentedMethodCall.MethodCall,
                 new ParsedSqlStatement(DatastoreVendor.CosmosDB, model, operation),
-                connectionInfo: endpoint != null ? new ConnectionInfo(endpoint.Host, endpoint.Port.ToString(), databaseName) : new ConnectionInfo(string.Empty, string.Empty, databaseName),
+                connectionInfo: endpoint != null ? new ConnectionInfo(DatastoreVendor.CosmosDB.ToKnownName(), endpoint.Host, endpoint.Port, databaseName) : new ConnectionInfo(string.Empty, string.Empty, string.Empty, databaseName),
                 commandText : querySpec != null ? _queryGetter.Invoke(querySpec) : string.Empty,
                 isLeaf: true);
 

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/CosmosDb/RequestInvokerHandlerWrapper.cs

@@ -67,7 +67,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
             var segment = transaction.StartDatastoreSegment(
                 instrumentedMethodCall.MethodCall,
                 new ParsedSqlStatement(DatastoreVendor.CosmosDB, model, operation),
-                connectionInfo: endpoint != null ? new ConnectionInfo(endpoint.Host, endpoint.Port.ToString(), databaseName) : new ConnectionInfo(string.Empty, string.Empty, databaseName),
+                connectionInfo: endpoint != null ? new ConnectionInfo(DatastoreVendor.CosmosDB.ToKnownName(), endpoint.Host, endpoint.Port, databaseName) : new ConnectionInfo(string.Empty, string.Empty, string.Empty, databaseName),
                 isLeaf: true);
 
             return Delegates.GetAsyncDelegateFor<Task>(agent, segment);