Skip to content

Commit 62bdb44

Browse files
tippmar-nrtippmar-nr
authored
fix: Complete array attribute serialization for all payload types (#3531)
1 parent 3e94fb9 commit 62bdb44

10 files changed

Lines changed: 495 additions & 18 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
using System;
5+
using System.Collections;
56
using System.Collections.Generic;
67
using System.Diagnostics;
78
using System.Linq;
@@ -121,6 +122,17 @@ public static object GenericConverter(object input)
121122
return span.TotalSeconds;
122123
case DateTimeOffset offset:
123124
return offset.ToString("o");
125+
case IEnumerable enumerable when !(input is string):
126+
var convertedList = new List<object>();
127+
foreach (var element in enumerable)
128+
{
129+
var converted = GenericConverter(element);
130+
if (converted != null)
131+
{
132+
convertedList.Add(converted);
133+
}
134+
}
135+
return convertedList;
124136
default:
125137
switch (Type.GetTypeCode(input.GetType()))
126138
{

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
using System;
5+
using System.Collections;
6+
using System.Collections.Generic;
7+
using System.Linq;
58
using NewRelic.Agent.Core.Attributes;
69

710
namespace NewRelic.Agent.Core.Segments;
@@ -31,6 +34,8 @@ public AttributeValue(IAttributeValue attribValue) : this(attribValue.AttributeD
3134

3235
public bool IsImmutable { get; private set; }
3336

37+
private List<object> _arrayValue;
38+
3439
private Lazy<object> _lazyValue;
3540
public Lazy<object> LazyValue
3641
{
@@ -42,6 +47,11 @@ public object Value
4247
{
4348
get
4449
{
50+
if (_arrayValue != null)
51+
{
52+
return _arrayValue;
53+
}
54+
4555
switch (ValueCase)
4656
{
4757
case ValueOneofCase.StringValue:
@@ -93,6 +103,9 @@ private void SetValue(object value)
93103
return;
94104
}
95105

106+
// Clear array value when setting a scalar value; overwritten below for IEnumerable
107+
_arrayValue = null;
108+
96109
if (value is string)
97110
{
98111
StringValue = (string)value;
@@ -129,6 +142,19 @@ private void SetValue(object value)
129142
return;
130143
}
131144

145+
// Note: this check relies on the string check earlier in this method to have already
146+
// returned, since string implements IEnumerable<char>.
147+
if (value is IEnumerable enumerable)
148+
{
149+
_arrayValue = value as List<object> ?? enumerable.Cast<object>().ToList();
150+
151+
// Set StringValue as fallback for the protobuf/infinite tracing serialization path,
152+
// which doesn't support array types in the oneof. This preserves the pre-array-support
153+
// behavior so the attribute is still captured rather than silently dropped.
154+
StringValue = value.ToString();
155+
return;
156+
}
157+
132158
switch (Type.GetTypeCode(value.GetType()))
133159
{
134160
case TypeCode.SByte:
@@ -156,4 +182,4 @@ private void SetValue(object value)
156182
break;
157183
}
158184
}
159-
}
185+
}

‎tests/Agent/IntegrationTests/Applications/AspNetCoreWebApiCustomAttributesApplication/Controllers/AttributeTestingController.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ public string CustomEmptyArrayAttributes()
8383
NewRelic.Api.Agent.NewRelic.GetAgent().CurrentTransaction.AddCustomAttribute("emptyArray", new string[] { });
8484
NewRelic.Api.Agent.NewRelic.GetAgent().CurrentTransaction.AddCustomAttribute("nullOnlyArray", new object[] { null, null });
8585

86+
// Attempt to force this as the captured transaction trace.
87+
System.Threading.Thread.Sleep(1000);
88+
8689
return "success";
8790
}
8891

@@ -93,6 +96,9 @@ public string CustomArrayWithNulls()
9396
NewRelic.Api.Agent.NewRelic.GetAgent().CurrentTransaction.AddCustomAttribute("arrayWithNulls", new object[] { "first", null, "third" });
9497
NewRelic.Api.Agent.NewRelic.GetAgent().CurrentTransaction.AddCustomAttribute("listAttribute", new List<string> { "list1", "list2", "list3" });
9598

99+
// Attempt to force this as the captured transaction trace.
100+
System.Threading.Thread.Sleep(1000);
101+
96102
return "success";
97103
}
98104

‎tests/Agent/IntegrationTests/Applications/CustomAttributesWebApi/MyController.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ public string CustomEmptyArrayAttributes()
8787
NewRelic.Api.Agent.NewRelic.GetAgent().CurrentTransaction.AddCustomAttribute("emptyArray", new string[] { });
8888
NewRelic.Api.Agent.NewRelic.GetAgent().CurrentTransaction.AddCustomAttribute("nullOnlyArray", new object[] { null, null });
8989

90+
// Attempt to force this as the captured transaction trace.
91+
Thread.Sleep(1000);
92+
9093
return "success";
9194
}
9295

@@ -102,4 +105,4 @@ public string CustomArrayWithNulls()
102105

103106
return "success";
104107
}
105-
}
108+
}

‎tests/Agent/IntegrationTests/IntegrationTests/CustomAttributes/AspNetCoreCustomAttributesArraySupport.cs‎

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,23 @@ public AspNetCoreCustomAttributesArraySupport(RemoteServiceFixtures.AspNetCoreWe
2727
configModifier.ForceTransactionTraces();
2828
configModifier.ConfigureFasterTransactionTracesHarvestCycle(10);
2929
configModifier.ConfigureFasterErrorTracesHarvestCycle(10);
30-
CommonUtils.ModifyOrCreateXmlAttributeInNewRelicConfig(configPath, new[] { "configuration", "log" }, "level", "debug");
30+
configModifier.ConfigureFasterSpanEventsHarvestCycle(10);
31+
configModifier.SetLogLevel("debug");
3132
},
3233
exerciseApplication: () =>
3334
{
35+
// Space each slow request across separate harvest cycles so each gets its own transaction trace.
3436
_fixture.GetCustomArrayAttributes();
37+
_fixture.AgentLog.WaitForLogLine(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1));
38+
3539
_fixture.GetCustomEmptyArrayAttributes();
40+
_fixture.AgentLog.WaitForLogLines(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1), 2);
41+
3642
_fixture.GetCustomArrayWithNulls();
43+
_fixture.AgentLog.WaitForLogLines(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1), 3);
3744

38-
_fixture.AgentLog.WaitForLogLine(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1));
45+
// Wait for span event data to be harvested
46+
_fixture.AgentLog.WaitForLogLine(AgentLogFile.SpanEventDataLogLineRegex, TimeSpan.FromMinutes(1));
3947
});
4048
_fixture.Initialize();
4149
}
@@ -92,6 +100,34 @@ public void Test_ArrayAttributes_AppearInTransactionEvent()
92100
);
93101
}
94102

103+
[Fact]
104+
public void Test_ArrayAttributes_AppearInSpanEvent()
105+
{
106+
var expectedTransactionName = @"WebTransaction/MVC/AttributeTesting/CustomArrayAttributes";
107+
108+
var spanEvents = _fixture.AgentLog.GetSpanEvents().ToList();
109+
var rootSpan = spanEvents.FirstOrDefault(se =>
110+
se.IntrinsicAttributes.ContainsKey("nr.entryPoint") &&
111+
se.IntrinsicAttributes["name"]?.ToString() == expectedTransactionName);
112+
113+
Assert.NotNull(rootSpan);
114+
115+
NrAssert.Multiple(
116+
() => Assertions.SpanEventHasAttributes(new Dictionary<string, object>
117+
{
118+
{ "stringArray", new[] { "red", "green", "blue" } }
119+
}, SpanEventAttributeType.User, rootSpan),
120+
() => Assertions.SpanEventHasAttributes(new Dictionary<string, object>
121+
{
122+
{ "intArray", new[] { 1, 2, 3, 4, 5 } }
123+
}, SpanEventAttributeType.User, rootSpan),
124+
() => Assertions.SpanEventHasAttributes(new Dictionary<string, object>
125+
{
126+
{ "boolArray", new[] { true, false, true } }
127+
}, SpanEventAttributeType.User, rootSpan)
128+
);
129+
}
130+
95131
[Fact]
96132
public void Test_EmptyAndNullOnlyArrays_AreSkipped()
97133
{
@@ -101,10 +137,18 @@ public void Test_EmptyAndNullOnlyArrays_AreSkipped()
101137

102138
Assert.NotNull(transactionEvent);
103139

140+
var transactionSample = _fixture.AgentLog.GetTransactionSamples()
141+
.Where(sample => sample.Path == expectedTransactionName)
142+
.FirstOrDefault();
143+
144+
Assert.NotNull(transactionSample);
145+
104146
// Verify empty/null arrays don't appear (consistent with our JsonSerializerHelpers behavior)
105147
NrAssert.Multiple(
106148
() => Assert.False(TransactionEventHasAttribute("emptyArray", transactionEvent), "Empty array should be skipped in transaction event"),
107-
() => Assert.False(TransactionEventHasAttribute("nullOnlyArray", transactionEvent), "Null-only array should be skipped in transaction event")
149+
() => Assert.False(TransactionEventHasAttribute("nullOnlyArray", transactionEvent), "Null-only array should be skipped in transaction event"),
150+
() => Assert.False(TransactionTraceHasAttribute("emptyArray", transactionSample), "Empty array should be skipped in transaction trace"),
151+
() => Assert.False(TransactionTraceHasAttribute("nullOnlyArray", transactionSample), "Null-only array should be skipped in transaction trace")
108152
);
109153
}
110154

@@ -127,12 +171,29 @@ public void Test_ArraysWithNulls_FilterNullElements()
127171
{ "listAttribute", new[] { "list1", "list2", "list3" } } // List<T> works as array
128172
}, TransactionEventAttributeType.User, transactionEvent)
129173
);
174+
175+
var transactionSample = _fixture.AgentLog.GetTransactionSamples()
176+
.Where(sample => sample.Path == expectedTransactionName)
177+
.FirstOrDefault();
178+
179+
Assert.NotNull(transactionSample);
180+
181+
NrAssert.Multiple(
182+
() => Assertions.TransactionTraceHasAttributes(new Dictionary<string, object>
183+
{
184+
{ "arrayWithNulls", new[] { "first", "third" } }
185+
}, TransactionTraceAttributeType.User, transactionSample),
186+
() => Assertions.TransactionTraceHasAttributes(new Dictionary<string, object>
187+
{
188+
{ "listAttribute", new[] { "list1", "list2", "list3" } }
189+
}, TransactionTraceAttributeType.User, transactionSample)
190+
);
130191
}
131192

132193
// Helper methods to check if attributes exist
133-
private bool TransactionTraceHasAttribute(string attributeName, dynamic transactionTrace)
194+
private bool TransactionTraceHasAttribute(string attributeName, TransactionSample sample)
134195
{
135-
return transactionTrace.TransactionTraceData.Attributes.UserAttributes.ContainsKey(attributeName);
196+
return sample.TraceData.Attributes.UserAttributes.ContainsKey(attributeName);
136197
}
137198

138199
private bool TransactionEventHasAttribute(string attributeName, dynamic transactionEvent)

‎tests/Agent/IntegrationTests/IntegrationTests/CustomAttributes/CustomAttributesArraySupport.cs‎

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,23 @@ public CustomAttributesArraySupport(RemoteServiceFixtures.CustomAttributesWebApi
2727
configModifier.ForceTransactionTraces();
2828
configModifier.ConfigureFasterTransactionTracesHarvestCycle(10);
2929
configModifier.ConfigureFasterErrorTracesHarvestCycle(10);
30-
CommonUtils.ModifyOrCreateXmlAttributeInNewRelicConfig(configPath, new[] { "configuration", "log" }, "level", "debug");
30+
configModifier.ConfigureFasterSpanEventsHarvestCycle(10);
31+
configModifier.SetLogLevel("debug");
3132
},
3233
exerciseApplication: () =>
3334
{
35+
// Space each slow request across separate harvest cycles so each gets its own transaction trace.
3436
_fixture.GetCustomArrayAttributes();
37+
_fixture.AgentLog.WaitForLogLine(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1));
38+
3539
_fixture.GetCustomEmptyArrayAttributes();
40+
_fixture.AgentLog.WaitForLogLines(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1), 2);
41+
3642
_fixture.GetCustomArrayWithNulls();
43+
_fixture.AgentLog.WaitForLogLines(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1), 3);
3744

38-
_fixture.AgentLog.WaitForLogLine(AgentLogFile.TransactionSampleLogLineRegex, TimeSpan.FromMinutes(1));
45+
// Wait for span event data to be harvested
46+
_fixture.AgentLog.WaitForLogLine(AgentLogFile.SpanEventDataLogLineRegex, TimeSpan.FromMinutes(1));
3947
});
4048
_fixture.Initialize();
4149
}
@@ -92,6 +100,34 @@ public void Test_ArrayAttributes_AppearInTransactionEvent()
92100
);
93101
}
94102

103+
[Fact]
104+
public void Test_ArrayAttributes_AppearInSpanEvent()
105+
{
106+
var expectedTransactionName = @"WebTransaction/WebAPI/My/CustomArrayAttributes";
107+
108+
var spanEvents = _fixture.AgentLog.GetSpanEvents().ToList();
109+
var rootSpan = spanEvents.FirstOrDefault(se =>
110+
se.IntrinsicAttributes.ContainsKey("nr.entryPoint") &&
111+
se.IntrinsicAttributes["name"]?.ToString() == expectedTransactionName);
112+
113+
Assert.NotNull(rootSpan);
114+
115+
NrAssert.Multiple(
116+
() => Assertions.SpanEventHasAttributes(new Dictionary<string, object>
117+
{
118+
{ "stringArray", new[] { "red", "green", "blue" } }
119+
}, SpanEventAttributeType.User, rootSpan),
120+
() => Assertions.SpanEventHasAttributes(new Dictionary<string, object>
121+
{
122+
{ "intArray", new[] { 1, 2, 3, 4, 5 } }
123+
}, SpanEventAttributeType.User, rootSpan),
124+
() => Assertions.SpanEventHasAttributes(new Dictionary<string, object>
125+
{
126+
{ "boolArray", new[] { true, false, true } }
127+
}, SpanEventAttributeType.User, rootSpan)
128+
);
129+
}
130+
95131
[Fact]
96132
public void Test_EmptyAndNullOnlyArrays_AreSkipped()
97133
{
@@ -101,10 +137,18 @@ public void Test_EmptyAndNullOnlyArrays_AreSkipped()
101137

102138
Assert.NotNull(transactionEvent);
103139

140+
var transactionSample = _fixture.AgentLog.GetTransactionSamples()
141+
.Where(sample => sample.Path == expectedTransactionName)
142+
.FirstOrDefault();
143+
144+
Assert.NotNull(transactionSample);
145+
104146
// Verify empty/null arrays don't appear (consistent with our JsonSerializerHelpers behavior)
105147
NrAssert.Multiple(
106148
() => Assert.False(TransactionEventHasAttribute("emptyArray", transactionEvent), "Empty array should be skipped in transaction event"),
107-
() => Assert.False(TransactionEventHasAttribute("nullOnlyArray", transactionEvent), "Null-only array should be skipped in transaction event")
149+
() => Assert.False(TransactionEventHasAttribute("nullOnlyArray", transactionEvent), "Null-only array should be skipped in transaction event"),
150+
() => Assert.False(TransactionTraceHasAttribute("emptyArray", transactionSample), "Empty array should be skipped in transaction trace"),
151+
() => Assert.False(TransactionTraceHasAttribute("nullOnlyArray", transactionSample), "Null-only array should be skipped in transaction trace")
108152
);
109153
}
110154

@@ -127,16 +171,33 @@ public void Test_ArraysWithNulls_FilterNullElements()
127171
{ "listAttribute", new[] { "list1", "list2", "list3" } } // List<T> works as array
128172
}, TransactionEventAttributeType.User, transactionEvent)
129173
);
174+
175+
var transactionSample = _fixture.AgentLog.GetTransactionSamples()
176+
.Where(sample => sample.Path == expectedTransactionName)
177+
.FirstOrDefault();
178+
179+
Assert.NotNull(transactionSample);
180+
181+
NrAssert.Multiple(
182+
() => Assertions.TransactionTraceHasAttributes(new Dictionary<string, object>
183+
{
184+
{ "arrayWithNulls", new[] { "first", "third" } }
185+
}, TransactionTraceAttributeType.User, transactionSample),
186+
() => Assertions.TransactionTraceHasAttributes(new Dictionary<string, object>
187+
{
188+
{ "listAttribute", new[] { "list1", "list2", "list3" } }
189+
}, TransactionTraceAttributeType.User, transactionSample)
190+
);
130191
}
131192

132193
// Helper methods to check if attributes exist
133-
private bool TransactionTraceHasAttribute(string attributeName, dynamic transactionTrace)
194+
private bool TransactionTraceHasAttribute(string attributeName, TransactionSample sample)
134195
{
135-
return transactionTrace.TransactionTraceData.Attributes.UserAttributes.ContainsKey(attributeName);
196+
return sample.TraceData.Attributes.UserAttributes.ContainsKey(attributeName);
136197
}
137198

138199
private bool TransactionEventHasAttribute(string attributeName, dynamic transactionEvent)
139200
{
140201
return transactionEvent.UserAttributes.ContainsKey(attributeName);
141202
}
142-
}
203+
}

0 commit comments

Comments
 (0)