Skip to content

Commit

Permalink
Fix runtime metric generation (#4531)
Browse files Browse the repository at this point in the history
* Update runtime metrics test to check for changes in committed memory

* Fix reading runtime metric

* PR feedback

* Try to reduce flake

* Logging for debugging flake

* Apparently this can be 0 in netcoreapp2.1?

* Skip assertions on < .NET Core 3.1 on non-Windows

* Stop sending CommittedMemory on < .NET Core 3.1 on non-Windows (because it always sends 0)

* Fix condition
  • Loading branch information
andrewlock authored Aug 22, 2023
1 parent ade24f6 commit dc5f664
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 1 deletion.
31 changes: 31 additions & 0 deletions tracer/src/Datadog.Trace/RuntimeMetrics/RuntimeMetricsWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ namespace Datadog.Trace.RuntimeMetrics
{
internal class RuntimeMetricsWriter : IDisposable
{
#if NETSTANDARD
// In < .NET Core 3.1 we don't send CommittedMemory, so we report differently on < .NET Core 3.1
private const string ProcessMetrics = $"{MetricsNames.ThreadsCount}, {MetricsNames.CpuUserTime}, {MetricsNames.CpuSystemTime}, {MetricsNames.CpuPercentage}";
#else
private const string ProcessMetrics = $"{MetricsNames.ThreadsCount}, {MetricsNames.CommittedMemory}, {MetricsNames.CpuUserTime}, {MetricsNames.CpuSystemTime}, {MetricsNames.CpuPercentage}";
#endif

private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<RuntimeMetricsWriter>();
private static readonly Func<IDogStatsd, TimeSpan, bool, IRuntimeMetricsListener> InitializeListenerFunc = InitializeListener;
Expand All @@ -29,6 +34,11 @@ internal class RuntimeMetricsWriter : IDisposable
private readonly IRuntimeMetricsListener _listener;

private readonly bool _enableProcessMetrics;
#if NETSTANDARD
// In .NET Core <3.1 on non-Windows, Process.PrivateMemorySize64 returns 0, so we disable this.
// https://github.com/dotnet/runtime/issues/23284
private readonly bool _enableProcessMemory = false;
#endif

private readonly ConcurrentDictionary<string, int> _exceptionCounts = new ConcurrentDictionary<string, int>();

Expand Down Expand Up @@ -63,6 +73,19 @@ internal RuntimeMetricsWriter(IDogStatsd statsd, TimeSpan delay, bool inAzureApp
_previousSystemCpu = systemCpu;

_enableProcessMetrics = true;
#if NETSTANDARD
// In .NET Core <3.1 on non-Windows, Process.PrivateMemorySize64 returns 0, so we disable this.
// https://github.com/dotnet/runtime/issues/23284
_enableProcessMemory = FrameworkDescription.Instance switch
{
{ } x when x.IsWindows() => true, // Works on Windows
{ } x when !x.IsCoreClr() => true, // Works on .NET Framework
_ when Environment.Version is { Major: >= 5 } => true, // Works on .NET 5 and above
_ when Environment.Version is { Major: 3, Minor: > 0 } => true, // 3.1 works
_ when Environment.Version is { Major: 3, Minor: 0 } => false, // 3.0 is broken on linux
_ => false, // everything else (i.e. <.NET Core 3.0) is broken
};
#endif
}
catch (Exception ex)
{
Expand Down Expand Up @@ -116,7 +139,15 @@ internal void PushEvents()

_statsd.Gauge(MetricsNames.ThreadsCount, threadCount);

#if NETSTANDARD
if (_enableProcessMemory)
{
_statsd.Gauge(MetricsNames.CommittedMemory, memoryUsage);
Log.Debug("Sent the following metrics to the DD agent: {Metrics}", MetricsNames.CommittedMemory);
}
#else
_statsd.Gauge(MetricsNames.CommittedMemory, memoryUsage);
#endif

// Get CPU time in milliseconds per second
_statsd.Gauge(MetricsNames.CpuUserTime, userCpu.TotalMilliseconds / _delay.TotalSeconds);
Expand Down
1 change: 1 addition & 0 deletions tracer/src/Datadog.Trace/Util/ProcessHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public static void GetCurrentProcessInformation(out string processName, out stri
public static void GetCurrentProcessRuntimeMetrics(out TimeSpan userProcessorTime, out TimeSpan systemCpuTime, out int threadCount, out long privateMemorySize)
{
var process = CurrentProcess.Instance;
process.Refresh();
userProcessorTime = process.UserProcessorTime;
systemCpuTime = process.PrivilegedProcessorTime;
threadCount = process.Threads.Count;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using Datadog.Trace.RuntimeMetrics;
using Datadog.Trace.TestHelpers;
using FluentAssertions;
using Xunit;
Expand Down Expand Up @@ -126,6 +127,44 @@ private void RunTest()
Assert.True(contentionRequestsCount > 0, "No contention metrics received. Metrics received: " + string.Join("\n", requests));
}

// using #if so it's a different test to the one we use in RuntimeMetricsWriter
#if NETFRAMEWORK || NETCOREAPP3_1_OR_GREATER
var runtimeIsBuggy = false;
#else
// https://github.com/dotnet/runtime/issues/23284
var runtimeIsBuggy = !EnvironmentTools.IsWindows();
#endif
if (runtimeIsBuggy)
{
requests.Should().NotContain(s => s.Contains(MetricsNames.CommittedMemory));
}
else
{
// these values shouldn't stay the same
var memoryRequests = requests
.Where(r => r.Contains(MetricsNames.CommittedMemory))
.Select(
r =>
{
_output.WriteLine($"Parsing metrics from {r}");
// parse to find the memory
var startIndex = r.IndexOf(MetricsNames.CommittedMemory, StringComparison.Ordinal);
var separator = r.IndexOf(':', startIndex + 1);
var endIndex = r.IndexOf('|', separator + 1);
var name = r.Substring(startIndex, separator - startIndex);
name.Should().Be(MetricsNames.CommittedMemory);
return long.Parse(r.Substring(separator + 1, endIndex - separator - 1));
})
.ToList();

if (memoryRequests.Count >= 2)
{
// skip the case where we only get one metric for some reason
// Don't require completely distinct to reduce flake
memoryRequests.Distinct().Should().NotHaveCount(1);
}
}

Assert.Empty(agent.Exceptions);
VerifyInstrumentation(processResult.Process);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ private static void Main()

Monitor.Enter(SyncRoot);

new Thread(GenerateMemoryPressure) { IsBackground = true }.Start();
new Thread(GenerateEvents) { IsBackground = true }.Start();

/*
Expand Down Expand Up @@ -47,7 +48,20 @@ private static void GenerateEvents()
}

// Sleep for 500ms while creating contention
Monitor.TryEnter(SyncRoot, 500);
Monitor.TryEnter(SyncRoot, 500);
}
}

private static void GenerateMemoryPressure()
{
while (true)
{
// Do some big allocating etc to ensure committed memory increases
// over time
var bigBuffer = new byte[100_000_000];
new Random().NextBytes(bigBuffer);

Thread.Sleep(5_000);
}
}
}
Expand Down

0 comments on commit dc5f664

Please sign in to comment.