From 662c1cd7d7bbdb5cb7ec23ebfe0e3de9bb9b9dc4 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 10 Dec 2024 16:11:41 -0800 Subject: [PATCH] More cleanup and code coverage --- .../NewRelic/Agent/Core/Segments/Segment.cs | 6 +- .../AwsSdk/ArnBuilder.cs | 5 +- .../UnitTests/CompositeTests/AgentApiTests.cs | 60 +++++++++++++++---- .../Core.UnitTest/Segments/SegmentTests.cs | 53 ++++++++++++++++ 4 files changed, 104 insertions(+), 20 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Core/Segments/Segment.cs b/src/Agent/NewRelic/Agent/Core/Segments/Segment.cs index 0e6d995c26..968e7d654b 100644 --- a/src/Agent/NewRelic/Agent/Core/Segments/Segment.cs +++ b/src/Agent/NewRelic/Agent/Core/Segments/Segment.cs @@ -451,13 +451,13 @@ public ISpan AddCustomAttribute(string key, object value) public ISpan AddCloudSdkAttribute(string key, object value) { - SpanAttributeValueCollection customAttribValues; + SpanAttributeValueCollection attribValues; lock (_attribValuesSyncRoot) { - customAttribValues = _attribValues ?? (_attribValues = new SpanAttributeValueCollection()); + attribValues = _attribValues ?? (_attribValues = new SpanAttributeValueCollection()); } - AttribDefs.GetCloudSdkAttribute(key).TrySetValue(customAttribValues, value); + AttribDefs.GetCloudSdkAttribute(key).TrySetValue(attribValues, value); return this; } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs index 4030057429..d40a53aaaa 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs @@ -119,11 +119,8 @@ public string BuildFromPartialLambdaArn(string invocationName) return null; } + // The member Region cannot be blank (it has a default) so we don't need to check it here region = !string.IsNullOrEmpty(region) ? region : Region; - if (string.IsNullOrEmpty(region)) - { - return null; - } if (!string.IsNullOrEmpty(alias)) { diff --git a/tests/Agent/UnitTests/CompositeTests/AgentApiTests.cs b/tests/Agent/UnitTests/CompositeTests/AgentApiTests.cs index 206275898b..5da764b13e 100644 --- a/tests/Agent/UnitTests/CompositeTests/AgentApiTests.cs +++ b/tests/Agent/UnitTests/CompositeTests/AgentApiTests.cs @@ -2211,10 +2211,10 @@ public void Test_GetLinkingMetadataReturnsValidValuesIfDTEnabled() #endregion GetLinkingMetadataCATS - #region Span Attributes + #region Span Custom Attributes [Test] - public void SpanAttributes() + public void SpanCustomAttributes() { var agentWrapperApi = _compositeTestAgent.GetAgent(); var dtm1 = DateTime.Now; @@ -2238,9 +2238,6 @@ public void SpanAttributes() segment.AddCustomAttribute("key7", dtm2); segment.AddCustomAttribute("key8", null); segment.AddCustomAttribute("", dtm2); - segment.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); - segment.AddCloudSdkAttribute("aws.region", "us-west-2"); - segment.AddCloudSdkAttribute("cloud.resource_id", "arn:aws:lambda:us-west-2:123456789012:function:myfunction"); var singleStringValue = new StringValues("avalue"); var multiStringValue = new StringValues(new[] { "onevalue", "twovalue", "threevalue" }); @@ -2260,13 +2257,6 @@ public void SpanAttributes() new ExpectedAttribute(){ Key = "key9b", Value = "onevalue,twovalue,threevalue"} }; - var expectedCloudSdkAttributes = new[] - { - new ExpectedAttribute(){ Key = "cloud.platform", Value = "aws_lambda"}, - new ExpectedAttribute(){ Key = "aws.region", Value = "us-west-2"}, - new ExpectedAttribute(){ Key = "cloud.resource_id", Value = "arn:aws:lambda:us-west-2:123456789012:function:myfunction"}, - }; - var unexpectedAttributes = new[] { "key8", @@ -2285,11 +2275,55 @@ public void SpanAttributes() ( () => Assert.That(allSpans, Has.Count.EqualTo(2)), () => SpanAssertions.HasAttributes(expectedAttributes, AttributeClassification.UserAttributes, testSpan), - () => SpanAssertions.HasAttributes(expectedCloudSdkAttributes, AttributeClassification.AgentAttributes, testSpan), () => SpanAssertions.DoesNotHaveAttributes(unexpectedAttributes, AttributeClassification.UserAttributes, testSpan) ); } #endregion + + #region Span Cloud SDK Attributes + [Test] + public void SpanCloudSdkAttributes() + { + var agentWrapperApi = _compositeTestAgent.GetAgent(); + var dtm1 = DateTime.Now; + var dtm2 = DateTimeOffset.Now; + + // ACT + var transaction = agentWrapperApi.CreateTransaction( + isWeb: true, + category: EnumNameCache.GetName(WebTransactionType.Action), + transactionDisplayName: "name", + doNotTrackAsUnitOfWork: true); + + var segment = agentWrapperApi.StartTransactionSegmentOrThrow("segment"); + + segment.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); + segment.AddCloudSdkAttribute("aws.region", "us-west-2"); + segment.AddCloudSdkAttribute("cloud.resource_id", "arn:aws:lambda:us-west-2:123456789012:function:myfunction"); + + var expectedAttributes = new[] + { + new ExpectedAttribute(){ Key = "cloud.platform", Value = "aws_lambda"}, + new ExpectedAttribute(){ Key = "aws.region", Value = "us-west-2"}, + new ExpectedAttribute(){ Key = "cloud.resource_id", Value = "arn:aws:lambda:us-west-2:123456789012:function:myfunction"}, + }; + + segment.End(); + transaction.End(); + + _compositeTestAgent.Harvest(); + + var allSpans = _compositeTestAgent.SpanEvents; + var testSpan = allSpans.LastOrDefault(); + + NrAssert.Multiple + ( + () => Assert.That(allSpans, Has.Count.EqualTo(2)), + () => SpanAssertions.HasAttributes(expectedAttributes, AttributeClassification.AgentAttributes, testSpan) + ); + } + + #endregion } } diff --git a/tests/Agent/UnitTests/Core.UnitTest/Segments/SegmentTests.cs b/tests/Agent/UnitTests/Core.UnitTest/Segments/SegmentTests.cs index 7a9e87fbe0..e54082e069 100644 --- a/tests/Agent/UnitTests/Core.UnitTest/Segments/SegmentTests.cs +++ b/tests/Agent/UnitTests/Core.UnitTest/Segments/SegmentTests.cs @@ -58,5 +58,58 @@ public void DurationOrZero_ReturnsDuration_IfDurationIsSet() Assert.That(duration, Is.EqualTo(TimeSpan.FromSeconds(1))); } + [Test] + public void Misc_Segment_Setters() + { + var segment = new Segment(TransactionSegmentStateHelpers.GetItransactionSegmentState(), new MethodCallData("Type", "Method", 1)); + Assert.That(segment.IsLeaf, Is.False); + segment.MakeLeaf(); + Assert.That(segment.IsLeaf, Is.True); + + Assert.That(segment.GetTransactionTraceName(), Is.Null); + segment.SetName("foo"); + Assert.That(segment.GetTransactionTraceName(), Is.EqualTo("foo")); + + Assert.That(segment.Combinable, Is.False); + segment.MakeCombinable(); + Assert.That(segment.Combinable, Is.True); + + Assert.That(segment.IsExternal, Is.False); + } + + [Test] + public void NoOpSegment() + { + var segment = new NoOpSegment(); + Assert.That(segment.IsDone, Is.True); + Assert.That(segment.IsValid, Is.False); + Assert.That(segment.IsDone, Is.True); + Assert.That(segment.DurationShouldBeDeductedFromParent, Is.False); + Assert.That(segment.IsLeaf, Is.False); + Assert.That(segment.IsExternal, Is.False); + Assert.That(segment.SpanId, Is.Null); + Assert.That(segment.SegmentData, Is.Not.Null); + Assert.That(segment.AttribDefs, Is.Not.Null); + Assert.That(segment.AttribValues, Is.Not.Null); + Assert.That(segment.TypeName, Is.EqualTo(string.Empty)); + Assert.That(segment.UserCodeFunction, Is.EqualTo(string.Empty)); + Assert.That(segment.UserCodeNamespace, Is.EqualTo(string.Empty)); + Assert.That(segment.SegmentNameOverride, Is.Null); + + Assert.DoesNotThrow(() => segment.End()); + Assert.DoesNotThrow(() => segment.End(new Exception())); + Assert.DoesNotThrow(() => segment.EndStackExchangeRedis()); + Assert.DoesNotThrow(() => segment.MakeCombinable()); + Assert.DoesNotThrow(() => segment.MakeLeaf()); + Assert.DoesNotThrow(() => segment.RemoveSegmentFromCallStack()); + Assert.DoesNotThrow(() => segment.SetMessageBrokerDestination("destination")); + Assert.DoesNotThrow(() => segment.SetSegmentData(null)); + Assert.DoesNotThrow(() => segment.AddCustomAttribute("key", "value")); + Assert.DoesNotThrow(() => segment.AddCloudSdkAttribute("key", "value")); + Assert.DoesNotThrow(() => segment.SetName("name")); + Assert.That(segment.GetCategory(), Is.EqualTo(string.Empty)); + Assert.That(segment.DurationOrZero, Is.EqualTo(TimeSpan.Zero)); + + } } }