Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Custom instrumentation supportability metrics #2797

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

feat: Custom instrumentation supportability metrics #2797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c8f287d
First pass at custom instrumentation supportability metrics
chynesNR Oct 3, 2024
0de881e
Merge branch 'main' into feature/custom-instrumentation-metrics
chynesNR Oct 3, 2024
931a039
Cleanup and unit test
chynesNR Oct 4, 2024
95be91a
Additional test path
chynesNR Oct 4, 2024
b48bdb4
Merge branch 'main' into feature/custom-instrumentation-metrics
chynesNR Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Agent/NewRelic/Agent/Core/AgentHealth/AgentHealthReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ public void ReportLibraryVersion(string assemblyName, string assemblyVersion)
TrySend(_metricBuilder.TryBuildLibraryVersionMetric(assemblyName, assemblyVersion));
}

public void ReportCustomInstrumentation(string assemblyName, string className, string method)
{
TrySend(_metricBuilder.TryBuildCustomInstrumentationMetric(assemblyName, className, method));
}

#region TransactionEvents

public void ReportTransactionEventReservoirResized(int newSize)
Expand Down
3 changes: 2 additions & 1 deletion src/Agent/NewRelic/Agent/Core/AgentHealth/IAgentHealthReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using Grpc.Core;
using NewRelic.Agent.Core.SharedInterfaces;
using NewRelic.Agent.Core.Transformers.TransactionTransformer;
using NewRelic.Agent.Extensions.Providers.Wrapper;
Expand All @@ -18,6 +17,8 @@ public interface IAgentHealthReporter : IOutOfBandMetricSource

void ReportLibraryVersion(string assemblyName, string assemblyVersion);

void ReportCustomInstrumentation(string assemblyName, string className, string method);

void ReportTransactionEventReservoirResized(int newSize);

void ReportTransactionEventCollected();
Expand Down
6 changes: 6 additions & 0 deletions src/Agent/NewRelic/Agent/Core/Metrics/MetricNames.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,12 @@ public static string GetSupportabilityLibraryVersion(string assemblyName, string
return SupportabilityLibraryVersionPs + assemblyName + PathSeparator + assemblyVersion;
}

private const string SupportabilityCustomInstrumentationPs = SupportabilityPs + "CustomInst" + PathSeparator;
public static string GetSupportabilityCustomInstrumentation(string assemblyName, string className, string method)
{
return SupportabilityCustomInstrumentationPs + assemblyName + PathSeparator + className + PathSeparator + method;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is what you were referring to when you mentioned the metric name is quite long? I wonder if we have an upper limit on the length of metric names? I don't have any good suggesions, though, for how you could make this any shorter and yet still be descriptive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, here's one from our integration tests: Supportability/CustomInst/DistributedTracingApiApplication/NewRelic.Agent.IntegrationTests.Applications.DistributedTracingApiApplication.Program/CallInsertDTHeaders
I think having the full class name would help with identifying things we want to instrument, but they are quite long. The spec says the limit is 255 bytes for a single metric, and 1MB for the payload. Most customers should only have a few custom instrumentation points (or usually none), so maybe it's ok.

}

// Utilization
private const string SupportabilityUtilizationPs = SupportabilityPs + "utilization" + PathSeparator;
private const string SupportabilityUtilizationBootIdError = SupportabilityUtilizationPs + "boot_id" + PathSeparator + "error";
Expand Down
2 changes: 2 additions & 0 deletions src/Agent/NewRelic/Agent/Core/WireModels/IMetricBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public interface IMetricBuilder

MetricWireModel TryBuildLibraryVersionMetric(string assemblyName, string assemblyVersion);

MetricWireModel TryBuildCustomInstrumentationMetric(string assemblyName, string className, string method);

MetricWireModel TryBuildMetricHarvestAttemptMetric();

MetricWireModel TryBuildTransactionEventReservoirResizedMetric();
Expand Down
7 changes: 6 additions & 1 deletion src/Agent/NewRelic/Agent/Core/WireModels/MetricWireModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,12 @@ public MetricWireModel TryBuildLibraryVersionMetric(string assemblyName, string
var data = MetricDataWireModel.BuildCountData();
return BuildMetric(_metricNameService, proposedName, null, data);
}

public MetricWireModel TryBuildCustomInstrumentationMetric(string assemblyName, string className, string method)
{
var proposedName = MetricNames.GetSupportabilityCustomInstrumentation(assemblyName, className, method);
var data = MetricDataWireModel.BuildCountData();
return BuildMetric(_metricNameService, proposedName, null, data);
}
public MetricWireModel TryBuildMetricHarvestAttemptMetric()
{
const string proposedName = MetricNames.SupportabilityMetricHarvestTransmit;
Expand Down
28 changes: 24 additions & 4 deletions src/Agent/NewRelic/Agent/Core/Wrapper/WrapperService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using NewRelic.Agent.Api;
using System.Reflection;
using System.Threading.Tasks;
using System.Collections.Generic;

namespace NewRelic.Agent.Core.Wrapper
{
Expand Down Expand Up @@ -43,6 +44,14 @@ public InstrumentedMethodInfoWrapper(InstrumentedMethodInfo instrumentedMethodIn
}

private readonly ConcurrentDictionary<ulong, InstrumentedMethodInfoWrapper> _functionIdToWrapper;
private readonly List<string> KnownCustomTracerNames = new List<string>
{
"NewRelic.Agent.Core.Tracer.Factories.BackgroundThreadTracerFactory",
"NewRelic.Agent.Core.Tracer.Factories.IgnoreTransactionTracerFactory",
"NewRelic.Providers.Wrapper.CustomInstrumentation.OtherTransactionWrapper",
"AsyncForceNewTransactionWrapper",
"OtherTransactionWrapper"
};

public WrapperService(IConfigurationService configurationService, IWrapperMap wrapperMap, IAgent agent, IAgentHealthReporter agentHealthReporter, IAgentTimerService agentTimerService)
{
Expand All @@ -68,9 +77,14 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(Type type, string methodNa
}
else
{
bool isCustom = KnownCustomTracerNames.Contains(tracerFactoryName);
var isAsync = TracerArgument.IsAsync(tracerArguments);

tracerFactoryName = ResolveTracerFactoryNameForAttributeInstrumentation(tracerArguments, isAsync, tracerFactoryName);
if (TracerArgument.IsFlagSet(tracerArguments, TracerFlags.AttributeInstrumentation))
{
tracerFactoryName = ResolveTracerFactoryNameForAttributeInstrumentation(tracerArguments, isAsync, tracerFactoryName);
isCustom = true;
}

var method = new Method(type, methodName, argumentSignature, functionId.GetHashCode());
var transactionNamePriority = TracerArgument.GetTransactionNamingPriority(tracerArguments);
Expand Down Expand Up @@ -108,7 +122,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(Type type, string methodNa
}

_functionIdToWrapper[functionId] = new InstrumentedMethodInfoWrapper(instrumentedMethodInfo, trackedWrapper);
GenerateLibraryVersionSupportabilityMetric(instrumentedMethodInfo);
GenerateSupportabilityMetrics(instrumentedMethodInfo, isCustom);
}

var wrapper = trackedWrapper.Wrapper;
Expand Down Expand Up @@ -228,7 +242,7 @@ private void HandleBeforeWrappedMethodException(ulong functionId, TrackedWrapper
}
}

private void GenerateLibraryVersionSupportabilityMetric(InstrumentedMethodInfo instrumentedMethodInfo)
private void GenerateSupportabilityMetrics(InstrumentedMethodInfo instrumentedMethodInfo, bool isCustom)
{
try
{
Expand All @@ -237,10 +251,16 @@ private void GenerateLibraryVersionSupportabilityMetric(InstrumentedMethodInfo i
var assemblyVersion = reflectionAssemblyName.Version.ToString();

_agentHealthReporter.ReportLibraryVersion(assemblyName, assemblyVersion);
if (isCustom)
{
string method = instrumentedMethodInfo.Method.MethodName;
string className = instrumentedMethodInfo.Method.Type.FullName;
_agentHealthReporter.ReportCustomInstrumentation(assemblyName, className, method);
}
}
catch (Exception ex)
{
Log.Error($"Failed to generate Library version Supportability Metric for {instrumentedMethodInfo.ToString()} : exception: {ex}");
Log.Error($"Failed to generate Supportability Metrics for {instrumentedMethodInfo.ToString()} : exception: {ex}");
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion tests/Agent/UnitTests/CompositeTests/WrapperServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class WrapperServiceTests
{
private static CompositeTestAgent _compositeTestAgent;
private IAgent _agent;
private const uint AttributeInstrumentation = 1 << 20;

[SetUp]
public void SetUp()
Expand Down Expand Up @@ -43,7 +44,7 @@ public void BeforeWrappedMethod_ReturnsNoOp_IfTheRequiredTransactionIsFinished()
using (var logging = new Logging())
{
var wrapperService = _compositeTestAgent.GetWrapperService();
var afterWrappedMethod = wrapperService.BeforeWrappedMethod(type, methodName, string.Empty, target, arguments, tracerFactoryName, null, 0, 0);
var afterWrappedMethod = wrapperService.BeforeWrappedMethod(type, methodName, string.Empty, target, arguments, tracerFactoryName, null, AttributeInstrumentation, 0);

Assert.Multiple(() =>
{
Expand Down
5 changes: 4 additions & 1 deletion tests/Agent/UnitTests/Core.UnitTest/Wrapper/WrapperService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class Class_WrapperService
{
private const uint EmptyTracerArgs = 0;
private const uint AsyncTracerArgs = 1 << 23;
private const uint AttributeInstrumentation = 1 << 20;

private WrapperService _wrapperService;

Expand Down Expand Up @@ -68,13 +69,15 @@ public void BeforeWrappedMethod_PassesCorrectParametersToWrapperLoader()
const string tracerFactoryName = "MyTracer";
var target = new object();
var arguments = new object[0];
_wrapperService.BeforeWrappedMethod(type, methodName, string.Empty, target, arguments, tracerFactoryName, null, EmptyTracerArgs, 0);
_wrapperService.BeforeWrappedMethod(type, methodName, string.Empty, target, arguments, tracerFactoryName, null, AttributeInstrumentation, 0);

var method = new Method(type, methodName, string.Empty);
var expectedMethodCall = new MethodCall(method, target, arguments, false);
var instrumetedMethodInfo = new InstrumentedMethodInfo(0, expectedMethodCall.Method, tracerFactoryName, false, null, null, false);

Mock.Assert(() => _wrapperMap.Get(instrumetedMethodInfo));
Mock.Assert(() => _agentHealthReporter.ReportLibraryVersion(Arg.IsAny<string>(), Arg.IsAny<string>()), Occurs.Once());
Mock.Assert(() => _agentHealthReporter.ReportCustomInstrumentation(Arg.IsAny<string>(), Arg.IsAny<string>(), Arg.IsAny<string>()), Occurs.Once());
}

[Test]
Expand Down
Loading