Skip to content

Commit 9500d4d

Browse files
chynesNRchynesNR
authored
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.
1 parent 1db5335 commit 9500d4d

38 files changed

+310
-76
lines changed

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

+50-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ public interface IAttributeDefinitions
4343
AttributeDefinition<float, double> DatabaseDuration { get; }
4444
AttributeDefinition<string, string> DbCollection { get; }
4545
AttributeDefinition<string, string> DbInstance { get; }
46+
AttributeDefinition<string, string> DbOperation { get; }
47+
AttributeDefinition<string, string> DbServerAddress { get; }
48+
AttributeDefinition<long, long> DbServerPort { get; }
4649
AttributeDefinition<string, string> DbStatement { get; }
50+
AttributeDefinition<string, string> DbSystem { get; }
4751
AttributeDefinition<string, string> DistributedTraceId { get; }
4852
AttributeDefinition<TimeSpan, double> Duration { get; }
4953
AttributeDefinition<bool, bool> IsErrorExpected { get; }
@@ -91,6 +95,8 @@ public interface IAttributeDefinitions
9195
AttributeDefinition<string, string> RequestUri { get; }
9296
AttributeDefinition<int?, string> ResponseStatus { get; }
9397
AttributeDefinition<bool, bool> Sampled { get; }
98+
AttributeDefinition<string, string> ServerAddress { get; }
99+
AttributeDefinition<long, long> ServerPort { get; }
94100
AttributeDefinition<SpanCategory, string> SpanCategory { get; }
95101
AttributeDefinition<string, string> SpanErrorClass { get; }
96102
AttributeDefinition<string, string> SpanErrorMessage { get; }
@@ -102,6 +108,7 @@ public interface IAttributeDefinitions
102108
AttributeDefinition<string, string> SyntheticsMonitorIdForTraces { get; }
103109
AttributeDefinition<string, string> SyntheticsResourceId { get; }
104110
AttributeDefinition<string, string> SyntheticsResourceIdForTraces { get; }
111+
AttributeDefinition<long, long> ThreadId { get; }
105112
AttributeDefinition<DateTime, long> Timestamp { get; }
106113
AttributeDefinition<DateTime, long> TimestampForError { get; }
107114
AttributeDefinition<TimeSpan, double> TotalTime { get; }
@@ -476,6 +483,12 @@ public AttributeDefinition<TypeAttributeValue, string> GetTypeAttribute(TypeAttr
476483
.AppliesTo(AttributeDestinations.SpanEvent)
477484
.Build(_attribFilter));
478485

486+
private AttributeDefinition<long, long> _threadId;
487+
public AttributeDefinition<long, long> ThreadId => _threadId ?? (_threadId =
488+
AttributeDefinitionBuilder.CreateLong("thread.id", AttributeClassification.Intrinsics)
489+
.AppliesTo(AttributeDestinations.SpanEvent)
490+
.Build(_attribFilter));
491+
479492
private AttributeDefinition<string, string> _component;
480493
public AttributeDefinition<string, string> Component => _component ?? (_component =
481494
AttributeDefinitionBuilder.CreateString("component", AttributeClassification.Intrinsics)
@@ -513,6 +526,18 @@ public AttributeDefinition<TypeAttributeValue, string> GetTypeAttribute(TypeAttr
513526
.AppliesTo(AttributeDestinations.SpanEvent)
514527
.Build(_attribFilter));
515528

529+
private AttributeDefinition<string, string> _dbSystem;
530+
public AttributeDefinition<string, string> DbSystem => _dbSystem ?? (_dbSystem =
531+
AttributeDefinitionBuilder.CreateString("db.system", AttributeClassification.AgentAttributes)
532+
.AppliesTo(AttributeDestinations.SpanEvent)
533+
.Build(_attribFilter));
534+
535+
private AttributeDefinition<string, string> _dbOperation;
536+
public AttributeDefinition<string, string> DbOperation => _dbOperation ?? (_dbOperation =
537+
AttributeDefinitionBuilder.CreateString("db.operation", AttributeClassification.AgentAttributes)
538+
.AppliesTo(AttributeDestinations.SpanEvent)
539+
.Build(_attribFilter));
540+
516541
private AttributeDefinition<string, string> _dbCollection;
517542
public AttributeDefinition<string, string> DbCollection => _dbCollection ?? (_dbCollection =
518543
AttributeDefinitionBuilder.CreateString("db.collection", AttributeClassification.AgentAttributes)
@@ -546,7 +571,31 @@ public AttributeDefinition<TypeAttributeValue, string> GetTypeAttribute(TypeAttr
546571

547572
private AttributeDefinition<string, string> _httpMethod;
548573
public AttributeDefinition<string, string> HttpMethod => _httpMethod ?? (_httpMethod =
549-
AttributeDefinitionBuilder.CreateString("http.method", AttributeClassification.AgentAttributes)
574+
AttributeDefinitionBuilder.CreateString("http.request.method", AttributeClassification.AgentAttributes)
575+
.AppliesTo(AttributeDestinations.SpanEvent)
576+
.Build(_attribFilter));
577+
578+
private AttributeDefinition<string, string> _serverAddress;
579+
public AttributeDefinition<string, string> ServerAddress => _serverAddress ?? (_serverAddress =
580+
AttributeDefinitionBuilder.CreateString("server.address", AttributeClassification.Intrinsics)
581+
.AppliesTo(AttributeDestinations.SpanEvent)
582+
.Build(_attribFilter));
583+
584+
private AttributeDefinition<long, long> _serverPort;
585+
public AttributeDefinition<long, long> ServerPort => _serverPort ?? (_serverPort =
586+
AttributeDefinitionBuilder.CreateLong("server.port", AttributeClassification.Intrinsics)
587+
.AppliesTo(AttributeDestinations.SpanEvent)
588+
.Build(_attribFilter));
589+
590+
private AttributeDefinition<string, string> _dbServerAddress;
591+
public AttributeDefinition<string, string> DbServerAddress => _dbServerAddress ?? (_dbServerAddress =
592+
AttributeDefinitionBuilder.CreateString("server.address", AttributeClassification.AgentAttributes)
593+
.AppliesTo(AttributeDestinations.SpanEvent)
594+
.Build(_attribFilter));
595+
596+
private AttributeDefinition<long, long> _dbServerPort;
597+
public AttributeDefinition<long, long> DbServerPort => _dbServerPort ?? (_dbServerPort =
598+
AttributeDefinitionBuilder.CreateLong("server.port", AttributeClassification.AgentAttributes)
550599
.AppliesTo(AttributeDestinations.SpanEvent)
551600
.Build(_attribFilter));
552601

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

+13-2
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,18 @@ namespace NewRelic.Agent.Core.Segments
2323
{
2424
public class DatastoreSegmentData : AbstractSegmentData, IDatastoreSegmentData
2525
{
26-
private readonly static ConnectionInfo EmptyConnectionInfo = new ConnectionInfo(null, null, null);
26+
private readonly static ConnectionInfo EmptyConnectionInfo = new ConnectionInfo(null, null, null, null);
2727

2828
public override SpanCategory SpanCategory => SpanCategory.Datastore;
2929

3030
public string Operation => _parsedSqlStatement.Operation;
3131
public DatastoreVendor DatastoreVendorName => _parsedSqlStatement.DatastoreVendor;
3232
public string Model => _parsedSqlStatement.Model;
3333
public string CommandText { get; set; }
34+
public string Vendor => _connectionInfo.Vendor;
3435
public string Host => _connectionInfo.Host;
36+
public int? Port => _connectionInfo.Port;
37+
public string PathOrId => _connectionInfo.PathOrId;
3538
public string PortPathOrId => _connectionInfo.PortPathOrId;
3639
public string DatabaseName => _connectionInfo.DatabaseName;
3740
public Func<object> GetExplainPlanResources { get; set; }
@@ -219,10 +222,18 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection
219222
AttribDefs.DbCollection.TrySetValue(attribVals, _parsedSqlStatement.Model);
220223
}
221224

225+
AttribDefs.DbSystem.TrySetValue(attribVals, Vendor);
222226
AttribDefs.DbInstance.TrySetValue(attribVals, DatabaseName);
227+
AttribDefs.DbOperation.TrySetValue(attribVals, Operation);
223228
AttribDefs.PeerAddress.TrySetValue(attribVals, $"{Host}:{PortPathOrId}");
224-
AttribDefs.PeerHostname.TrySetValue(attribVals, Host);
225229
AttribDefs.SpanKind.TrySetDefault(attribVals);
230+
// peer.hostname and server.address must match
231+
AttribDefs.PeerHostname.TrySetValue(attribVals, Host);
232+
AttribDefs.DbServerAddress.TrySetValue(attribVals, Host);
233+
if (Port != null)
234+
{
235+
AttribDefs.DbServerPort.TrySetValue(attribVals, Port.Value);
236+
}
226237
}
227238

228239
public void SetConnectionInfo(ConnectionInfo connInfo)

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

+2
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection
9393
AttribDefs.Component.TrySetValue(attribVals, _segmentState.TypeName);
9494
AttribDefs.SpanKind.TrySetDefault(attribVals);
9595
AttribDefs.HttpStatusCode.TrySetValue(attribVals, _httpStatusCode); //Attrib handles null
96+
AttribDefs.ServerAddress.TrySetValue(attribVals, Uri.Host);
97+
AttribDefs.ServerPort.TrySetValue(attribVals, Uri.Port);
9698
}
9799

98100
public override string GetTransactionTraceName()

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

+11
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public Segment(ITransactionSegmentState transactionSegmentState, MethodCallData
5353
Data.AttachSegmentDataState(this);
5454
Combinable = false;
5555
IsLeaf = false;
56+
IsAsync = methodCallData.IsAsync;
5657
}
5758

5859
/// <summary>
@@ -75,6 +76,7 @@ public Segment(ITransactionSegmentState transactionSegmentState, MethodCallData
7576
Data.AttachSegmentDataState(this);
7677
Combinable = false;
7778
IsLeaf = true;
79+
IsAsync = methodCallData.IsAsync;
7880
}
7981

8082
/// <summary>
@@ -105,6 +107,7 @@ public Segment(TimeSpan relativeStartTime, TimeSpan? duration, Segment segment,
105107
}
106108

107109
SpanId = segment.SpanId;
110+
IsAsync = segment.IsAsync;
108111
}
109112

110113
public bool IsDone
@@ -229,6 +232,8 @@ public void RemoveSegmentFromCallStack()
229232
/// </summary>
230233
public int ThreadId { get; private set; }
231234

235+
public bool IsAsync { get; private set; }
236+
232237
// used to set the ["unfinished"] parameter, not for real-time state of segment
233238
public bool Unfinished { get; private set; }
234239

@@ -335,6 +340,11 @@ public SpanAttributeValueCollection GetAttributeValues()
335340
AttribDefs.CodeFunction.TrySetValue(attribValues, codeFunction);
336341
}
337342

343+
if (!IsAsync)
344+
{
345+
AttribDefs.ThreadId.TrySetValue(attribValues, ThreadId);
346+
}
347+
338348
Data.SetSpanTypeSpecificAttributes(attribValues);
339349

340350
return attribValues;
@@ -441,5 +451,6 @@ public string GetCategory()
441451
{
442452
return EnumNameCache<SpanCategory>.GetName(Data.SpanCategory);
443453
}
454+
444455
}
445456
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -332,13 +332,13 @@ private static MethodCallData GetMethodCallData(MethodCall methodCall)
332332
var typeName = methodCall.Method.Type.FullName ?? "[unknown]";
333333
var methodName = methodCall.Method.MethodName;
334334
var invocationTargetHashCode = RuntimeHelpers.GetHashCode(methodCall.InvocationTarget);
335-
return new MethodCallData(typeName, methodName, invocationTargetHashCode);
335+
return new MethodCallData(typeName, methodName, invocationTargetHashCode, methodCall.IsAsync);
336336
}
337337

338338
// Used for StackExchange.Redis since we will not be instrumenting any methods when creating the many DataStore segments
339339
private static MethodCallData GetMethodCallData(string typeName, string methodName, int invocationTargetHashCode)
340340
{
341-
return new MethodCallData(typeName, methodName, invocationTargetHashCode);
341+
return new MethodCallData(typeName, methodName, invocationTargetHashCode, true); // assume async
342342
}
343343

344344
private static MetricNames.MessageBrokerDestinationType AgentWrapperApiEnumToMetricNamesEnum(

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@ public class MethodCallData
88
public readonly string TypeName;
99
public readonly string MethodName;
1010
public readonly int InvocationTargetHashCode;
11+
public readonly bool IsAsync;
1112

12-
public MethodCallData(string typeName, string methodName, int invocationTargetHashCode)
13+
public MethodCallData(string typeName, string methodName, int invocationTargetHashCode, bool isAsync = false)
1314
{
1415
TypeName = typeName;
1516
MethodName = methodName;
1617
InvocationTargetHashCode = invocationTargetHashCode;
18+
IsAsync = isAsync;
1719
}
1820

1921
public override string ToString()
@@ -27,6 +29,7 @@ public override int GetHashCode()
2729
hash = hash * 23 + TypeName.GetHashCode();
2830
hash = hash * 23 + MethodName.GetHashCode();
2931
hash = hash * 23 + InvocationTargetHashCode;
32+
hash = hash * 29 + IsAsync.GetHashCode();
3033
return hash;
3134
}
3235

@@ -37,7 +40,8 @@ public override bool Equals(object obj)
3740
var other = (MethodCallData)obj;
3841
return InvocationTargetHashCode == other.InvocationTargetHashCode &&
3942
MethodName.Equals(other.MethodName) &&
40-
TypeName.Equals(other.TypeName);
43+
TypeName.Equals(other.TypeName) &&
44+
IsAsync == other.IsAsync;
4145
}
4246
return false;
4347
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(Type type, string methodNa
142142
}
143143
}
144144

145-
var methodCall = new MethodCall(instrumentedMethodInfo.Method, invocationTarget, methodArguments);
145+
var methodCall = new MethodCall(instrumentedMethodInfo.Method, invocationTarget, methodArguments, instrumentedMethodInfo.IsAsync);
146146
var instrumentedMethodCall = new InstrumentedMethodCall(methodCall, instrumentedMethodInfo);
147147

148148
// 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

+21-3
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,25 @@ namespace NewRelic.Agent.Extensions.Parsing
55
{
66
public class ConnectionInfo
77
{
8-
public ConnectionInfo(string host, string portPathOrId, string databaseName, string instanceName = null)
8+
public ConnectionInfo(string vendor, string host, int port, string databaseName, string instanceName = null)
99
{
10+
Vendor = vendor;
1011
Host = ValueOrUnknown(host);
11-
PortPathOrId = ValueOrUnknown(portPathOrId);
12+
if (port >= 0)
13+
{
14+
Port = port;
15+
}
16+
PathOrId = ValueOrUnknown(string.Empty);
17+
DatabaseName = ValueOrUnknown(databaseName);
18+
InstanceName = instanceName;
19+
}
20+
21+
public ConnectionInfo(string vendor, string host, string pathOrId, string databaseName, string instanceName = null)
22+
{
23+
Vendor = vendor;
24+
Host = ValueOrUnknown(host);
25+
Port = null;
26+
PathOrId = ValueOrUnknown(pathOrId);
1227
DatabaseName = ValueOrUnknown(databaseName);
1328
InstanceName = instanceName;
1429
}
@@ -18,8 +33,11 @@ private static string ValueOrUnknown(string value)
1833
return string.IsNullOrEmpty(value) ? "unknown" : value;
1934
}
2035

36+
public string Vendor { get; private set; }
2137
public string Host { get; private set; }
22-
public string PortPathOrId { get; private set; }
38+
public string PortPathOrId { get => (Port != null) ? Port.ToString() : PathOrId; }
39+
public int? Port { get; private set; } = null;
40+
public string PathOrId { get; private set; } = string.Empty;
2341
public string DatabaseName { get; private set; }
2442
public string InstanceName { get; private set; }
2543
}

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

+18
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,24 @@ public enum DatastoreVendor
8282
Other
8383
}
8484

85+
public static class DatastoreVendorExtensions
86+
{
87+
// Convert our internal enum to the matching OTel "known" name for a database provider
88+
public static string ToKnownName(this DatastoreVendor vendor)
89+
{
90+
switch (vendor)
91+
{
92+
case DatastoreVendor.Other:
93+
return "other_sql";
94+
case DatastoreVendor.IBMDB2:
95+
return "db2";
96+
// The others match our enum name
97+
default:
98+
return EnumNameCache<DatastoreVendor>.GetNameToLower(vendor);
99+
}
100+
}
101+
}
102+
85103
public static class EnumNameCache<TEnum> // c# 7.3: where TEnum : System.Enum
86104
{
87105
private static readonly ConcurrentDictionary<TEnum, string> Cache = new ConcurrentDictionary<TEnum, string>();

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ public class MethodCall
1111
public readonly Method Method;
1212
public readonly object InvocationTarget;
1313
public readonly object[] MethodArguments;
14+
public readonly bool IsAsync;
1415

15-
public MethodCall(Method method, object invocationTarget, object[] methodArguments)
16+
public MethodCall(Method method, object invocationTarget, object[] methodArguments, bool isAsync)
1617
{
1718
Method = method;
1819
InvocationTarget = invocationTarget;
1920
MethodArguments = methodArguments ?? new object[0];
21+
IsAsync = isAsync;
2022
}
2123
}
2224
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ private ISegment SetupSegment(ITransaction transaction, HttpContext context)
148148
{
149149
// Seems like it would be cool to not require all of this for a segment???
150150
var method = new Method(typeof(WrapPipelineMiddleware), nameof(Invoke), nameof(context));
151-
var methodCall = new MethodCall(method, this, new object[] { context });
151+
var methodCall = new MethodCall(method, this, new object[] { context }, true);
152152

153153
var segment = transaction.StartTransactionSegment(methodCall, "Middleware Pipeline");
154154
return segment;

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
7878
var segment = transaction.StartDatastoreSegment(
7979
instrumentedMethodCall.MethodCall,
8080
new ParsedSqlStatement(DatastoreVendor.CosmosDB, model, operation),
81-
connectionInfo: endpoint != null ? new ConnectionInfo(endpoint.Host, endpoint.Port.ToString(), databaseName) : new ConnectionInfo(string.Empty, string.Empty, databaseName),
81+
connectionInfo: endpoint != null ? new ConnectionInfo(DatastoreVendor.CosmosDB.ToKnownName(), endpoint.Host, endpoint.Port, databaseName) : new ConnectionInfo(string.Empty, string.Empty, string.Empty, databaseName),
8282
commandText : querySpec != null ? _queryGetter.Invoke(querySpec) : string.Empty,
8383
isLeaf: true);
8484

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
6767
var segment = transaction.StartDatastoreSegment(
6868
instrumentedMethodCall.MethodCall,
6969
new ParsedSqlStatement(DatastoreVendor.CosmosDB, model, operation),
70-
connectionInfo: endpoint != null ? new ConnectionInfo(endpoint.Host, endpoint.Port.ToString(), databaseName) : new ConnectionInfo(string.Empty, string.Empty, databaseName),
70+
connectionInfo: endpoint != null ? new ConnectionInfo(DatastoreVendor.CosmosDB.ToKnownName(), endpoint.Host, endpoint.Port, databaseName) : new ConnectionInfo(string.Empty, string.Empty, string.Empty, databaseName),
7171
isLeaf: true);
7272

7373
return Delegates.GetAsyncDelegateFor<Task>(agent, segment);

0 commit comments

Comments
 (0)