Skip to content

Commit 8e32697

Browse files
committed
* Add http.statusText attribute on spanEvent (optional)
* Update ActivityBridge to handle Client external http activity kind correctly
1 parent 4399bc6 commit 8e32697

13 files changed

Lines changed: 132 additions & 17 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public interface IAttributeDefinitions
6363
AttributeDefinition<string, string> HostDisplayName { get; }
6464
AttributeDefinition<string, string> HttpMethod { get; }
6565
AttributeDefinition<long?, long> HttpStatusCode { get; }
66+
AttributeDefinition<string, string> HttpStatusText { get; }
6667
AttributeDefinition<Uri, string> HttpUrl { get; }
6768
AttributeDefinition<bool, bool> IsError { get; }
6869
AttributeDefinition<string, string> NameForSpan { get; }
@@ -439,6 +440,17 @@ public AttributeDefinition<TypeAttributeValue, string> GetTypeAttribute(TypeAttr
439440
.WithConvert(x => x.GetValueOrDefault()) //This is ok b/c we check for null input earlier
440441
.Build(_attribFilter));
441442

443+
private AttributeDefinition<string, string> _httpStatusText;
444+
public AttributeDefinition<string, string> HttpStatusText => _httpStatusText ??=
445+
AttributeDefinitionBuilder.CreateString("http.statusText", AttributeClassification.AgentAttributes)
446+
.AppliesTo(AttributeDestinations.ErrorEvent)
447+
.AppliesTo(AttributeDestinations.ErrorTrace)
448+
.AppliesTo(AttributeDestinations.TransactionEvent)
449+
.AppliesTo(AttributeDestinations.ErrorEvent)
450+
.AppliesTo(AttributeDestinations.SpanEvent)
451+
.AppliesTo(AttributeDestinations.TransactionTrace)
452+
.Build(_attribFilter);
453+
442454
private AttributeDefinition<string, string> _clientCrossProcessId;
443455
public AttributeDefinition<string, string> ClientCrossProcessId => _clientCrossProcessId ?? (_clientCrossProcessId =
444456
AttributeDefinitionBuilder.CreateString("client_cross_process_id", AttributeClassification.Intrinsics)

src/Agent/NewRelic/Agent/Core/OpenTelemetryBridge/ActivityBridge.cs

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313
using NewRelic.Agent.Core.Errors;
1414
using NewRelic.Agent.Core.Segments;
1515
using NewRelic.Agent.Core.Transactions;
16+
using NewRelic.Agent.Core.Utilities;
1617
using NewRelic.Agent.Extensions.Api.Experimental;
1718
using NewRelic.Agent.Extensions.Logging;
1819
using NewRelic.Agent.Extensions.Providers.Wrapper;
20+
using NewRelic.Agent.Extensions.SystemExtensions.Collections.Generic;
1921

2022
namespace NewRelic.Agent.Core.OpenTelemetryBridge
2123
{
@@ -67,7 +69,7 @@ private bool TryCreateActivityListener()
6769
// TODO: Enforce that an appropriate minimum version of the DiagnosticSource assembly is loaded.
6870
if (assembly == null || (assembly.GetName().Version?.Major ?? 0) < 7)
6971
{
70-
Log.Debug("DiagnosticSource assembly version < 7 is not compatible with OpenTelemetry Bridge. Not starting the activity listener.");
72+
Log.Debug("DiagnosticSource assembly not found or version < 7 is not compatible with OpenTelemetry Bridge. Not starting the activity listener.");
7173
return false;
7274
}
7375

@@ -494,7 +496,7 @@ private static void ActivityStopped(object originalActivity, IAgent agent, IErro
494496

495497
if (segment != null)
496498
{
497-
AddActivityTagsToSegment(originalActivity, segment);
499+
AddActivityTagsToSegment(originalActivity, segment, agent);
498500
AddExceptionEventInformationToSegment(originalActivity, segment, errorService);
499501
segment.End();
500502
}
@@ -506,14 +508,79 @@ private static void ActivityStopped(object originalActivity, IAgent agent, IErro
506508
}
507509
}
508510

509-
private static void AddActivityTagsToSegment(object originalActivity, ISegment segment)
511+
private static void AddActivityTagsToSegment(object originalActivity, ISegment segment, IAgent agent)
510512
{
511513
dynamic activity = originalActivity;
512-
foreach (var tag in activity.TagObjects)
514+
ActivityKind activityKind = (ActivityKind)activity.Kind;
515+
var tags = ((IEnumerable<KeyValuePair<string, object>>)activity.TagObjects).ToDictionary(t => t.Key, t => t.Value);
516+
if (tags.Count == 0)
513517
{
514-
// TODO: We may not want to add all tags to the segment. We may want to filter out some tags, especially
515-
// the ones that we map to intrinsic or agent attributes.
516-
segment.AddCustomAttribute((string)tag.Key, (object)tag.Value);
518+
Log.Debug($"Activity {activity.Id} has no tags. Not adding tags to segment.");
519+
return;
520+
}
521+
522+
// based on activity kind, create the appropriate segment data
523+
switch (activityKind)
524+
{
525+
case ActivityKind.Client:
526+
// could be an http call or a database call, so need to look for specific tags to decide
527+
if (tags.TryGetValue<string, string, object>("http.request.method", out var method) || tags.TryGetValue<string, string, object>("http.method", out method)) // it's an HTTP call
528+
{
529+
if (!tags.TryGetValue<string, string, object>("url.full", out var url) && !tags.TryGetValue<string, string, object>("http.url", out url))
530+
{
531+
Log.Debug($"Activity {activity.Id} with Activity.Kind {activityKind} is missing required tags for url. Not creating an ExternalSegmentData.");
532+
break;
533+
}
534+
if (string.IsNullOrEmpty(url) || string.IsNullOrEmpty(method))
535+
{
536+
Log.Debug($"Activity {activity.Id} with Activity.Kind {activityKind} is missing required tags for url and method. Not creating an ExternalSegmentData.");
537+
break;
538+
}
539+
540+
Uri uri = new Uri(url);
541+
var externalSegmentData = new ExternalSegmentData(uri, method);
542+
// set the http status code
543+
if (tags.TryGetValue<int, string, object>("http.response.status_code", out var statusCode) || tags.TryGetValue<int, string, object>("http.status_code", out statusCode))
544+
{
545+
tags.TryGetValue<string, string, object>("http.status_text", out var statusText);
546+
externalSegmentData.SetHttpStatus(statusCode, statusText);
547+
}
548+
549+
// check for AWS
550+
if (!string.IsNullOrEmpty(agent.Configuration.AwsAccountId))
551+
{
552+
if (tags.TryGetValue<string, string, object>("faas.invoked_name", out var awsName))
553+
segment.AddCloudSdkAttribute("cloud.platform", "aws_lambda");
554+
if (tags.TryGetValue<string, string, object>("faas.invoked_provider", out var awsProvider) && tags.TryGetValue<string, string, object>("aws.region", out var awsRegion))
555+
segment.AddCloudSdkAttribute("cloud.resource_id", $"arn:aws:lambda:{awsRegion}:{agent.Configuration.AwsAccountId}:function:{awsName}");
556+
}
557+
558+
segment.GetExperimentalApi().SetSegmentData(externalSegmentData);
559+
}
560+
else if (tags.TryGetValue("db.system", out var dbType)) // it's a database call
561+
{
562+
// TODO: need IDatabaseService here so we can create a DatastoreSegmentData instance.
563+
//var dbSegmentData = new DatastoreSegmentData( );
564+
//segment.GetExperimentalApi().SetSegmentData(dbSegmentData);
565+
}
566+
else
567+
{
568+
Log.Debug($"Activity {activity.Id} with Activity.Kind {activityKind} is missing required tags to determine whether it's an HTTP or database activity.");
569+
}
570+
break;
571+
case ActivityKind.Internal:
572+
case ActivityKind.Server:
573+
case ActivityKind.Producer:
574+
case ActivityKind.Consumer:
575+
default:
576+
// as a fallback, add all tags to the segment as custom attributes if we did not create a specific segment data type.
577+
foreach (var tag in tags)
578+
{
579+
// TODO: We may not want to add all tags to the segment. We may want to filter out some tags, especially
580+
// the ones that we map to intrinsic or agent attributes.
581+
segment.AddCustomAttribute(tag.Key, tag.Value);
582+
}
583+
break;
517584
}
518585
}
519586

@@ -674,6 +741,8 @@ public RuntimeNewRelicActivity(object activity)
674741
_dynamicActivity = (dynamic)_activity;
675742
}
676743

744+
public dynamic DynamicActivity => _dynamicActivity;
745+
677746
public bool IsStopped => (bool?)(_dynamicActivity)?.IsStopped ?? true;
678747

679748
public string SpanId => (string)(_dynamicActivity)?.SpanId.ToString();
@@ -682,6 +751,12 @@ public RuntimeNewRelicActivity(object activity)
682751

683752
public string DisplayName => (string)(_dynamicActivity)?.DisplayName;
684753

754+
/// <summary>
755+
/// Gets the ActivityKind for the activity. Can be safely cast to ActivityKind when needed.
756+
/// Defaults to Internal if there's no dynamic activity
757+
/// </summary>
758+
public int Kind => (int?)(_dynamicActivity)?.Kind ?? (int)ActivityKind.Internal;
759+
685760
public void Dispose()
686761
{
687762
_dynamicActivity?.Dispose();

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public class ExternalSegmentData : AbstractSegmentData, IExternalSegmentData
2020
private const string TransactionGuidSegmentParameterKey = "transaction_guid";
2121

2222
private int? _httpStatusCode;
23+
private string _httpStatusText;
2324

2425
public override SpanCategory SpanCategory => SpanCategory.Http;
2526

@@ -36,9 +37,10 @@ public ExternalSegmentData(Uri uri, string method, CrossApplicationResponseData
3637

3738
public CrossApplicationResponseData CrossApplicationResponseData { get; set; }
3839

39-
public void SetHttpStatusCode(int httpStatusCode)
40+
public void SetHttpStatus(int httpStatusCode, string httpStatusText = null)
4041
{
4142
_httpStatusCode = httpStatusCode;
43+
_httpStatusText = httpStatusText;
4244
}
4345

4446
internal override IEnumerable<KeyValuePair<string, object>> Finish()
@@ -92,6 +94,7 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection
9294
AttribDefs.Component.TrySetValue(attribVals, _segmentState.TypeName);
9395
AttribDefs.SpanKind.TrySetDefault(attribVals);
9496
AttribDefs.HttpStatusCode.TrySetValue(attribVals, _httpStatusCode); //Attrib handles null
97+
AttribDefs.HttpStatusText.TrySetValue(attribVals, _httpStatusText);
9598
AttribDefs.ServerAddress.TrySetValue(attribVals, Uri.Host);
9699
AttribDefs.ServerPort.TrySetValue(attribVals, Uri.Port);
97100
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,11 @@ private Segment StartSegmentImpl(string typeName, string methodName, int invocat
261261
public ISegment StartActivitySegment(MethodCall methodCall, INewRelicActivity activity)
262262
{
263263
var segment = StartSegmentImpl(methodCall, activity);
264-
var segmentData = new SimpleSegmentData(activity.DisplayName);
265264

266-
segment.SetSegmentData(segmentData);
265+
// create a simple segment data with the activity's display name
266+
// this will likely get replaced with a more specific segment data when the activity is stopped
267+
var simpleSegmentData = new SimpleSegmentData(activity.DisplayName);
268+
segment.SetSegmentData(simpleSegmentData);
267269

268270
if (Log.IsFinestEnabled) LogFinest($"Segment start {{{segment.ToStringForFinestLogging()}}}");
269271

src/Agent/NewRelic/Agent/Core/Utilities/DictionaryExtensions.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,5 +98,20 @@ public static ReadOnlyDictionary<TKey, TValue> CopyToReadOnly<TKey, TValue>(this
9898
{
9999
return new ReadOnlyDictionary<TKey, TValue>(new Dictionary<TKey, TValue>(source));
100100
}
101+
102+
/// <summary>
103+
/// Attempts to get a value from the dictionary and cast it to the specified type.
104+
/// </summary>
105+
public static bool TryGetValue<TResult, TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, out TResult defaultValue)
106+
{
107+
if (dictionary.TryGetValue(key, out var value) && value is TResult typedValue)
108+
{
109+
defaultValue = typedValue;
110+
return true;
111+
}
112+
113+
defaultValue = default;
114+
return false;
115+
}
101116
}
102117
}

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/Experimental/IExternalSegmentData.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ namespace NewRelic.Agent.Api.Experimental
99
/// </summary>
1010
public interface IExternalSegmentData : ISegmentData
1111
{
12-
void SetHttpStatusCode(int httpStatusCode);
12+
void SetHttpStatus(int httpStatusCode, string httpStatusText = null);
1313
}
1414
}

src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/Experimental/INewRelicActivity.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,17 @@ namespace NewRelic.Agent.Extensions.Api.Experimental
88
{
99
public interface INewRelicActivity : IDisposable
1010
{
11+
/// <summary>
12+
/// Gets the underlying activity as a dynamic type.
13+
/// Provides a way to access properties that are not directly exposed in this interface
14+
/// </summary>
15+
dynamic DynamicActivity { get; }
16+
1117
string SpanId { get; }
1218
string TraceId { get; }
1319
string DisplayName { get; }
1420
bool IsStopped { get; }
21+
int Kind { get; }
1522

1623
// can't use a Segment {get; set;} property here because it causes a circular reference between Activity and Segment
1724
void SetSegment(ISegment segment);

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/HttpClient/SendAsync.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ private static void TryProcessResponse(IAgent agent, HttpResponseMessage respons
174174
}
175175

176176
var httpStatusCode = response.StatusCode;
177-
externalSegmentData.SetHttpStatusCode((int)httpStatusCode);
177+
externalSegmentData.SetHttpStatus((int)httpStatusCode);
178178

179179
// Everything after this is for CAT, so bail if we're not using it
180180
if (agent.Configuration.DistributedTracingEnabled || !agent.Configuration.CrossApplicationTracingEnabled)

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/HttpWebRequest/GetResponseWrapper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private static void TryProcessResponse(WebResponse response, ITransaction transa
6565
var statusCode = httpWebResponse?.StatusCode;
6666
if (statusCode.HasValue)
6767
{
68-
externalSegmentData.SetHttpStatusCode((int)statusCode.Value);
68+
externalSegmentData.SetHttpStatus((int)statusCode.Value);
6969
}
7070

7171
var headers = response?.Headers?.ToDictionary();

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RestSharp/ExecuteTaskAsync.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ private static void TryProcessResponse(IAgent agent, Task responseTask, ITransac
7272
var statusCode = RestSharpHelper.GetResponseStatusCode(restResponse);
7373
if (statusCode != 0)
7474
{
75-
externalSegmentData.SetHttpStatusCode(statusCode);
75+
externalSegmentData.SetHttpStatus(statusCode);
7676
}
7777

7878
var headers = RestSharpHelper.GetResponseHeaders(restResponse);

0 commit comments

Comments
 (0)