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.

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

[Instrumentation.Process] Added the metrics for CpuTime and CpuUtilization. #625

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
118 changes: 105 additions & 13 deletions src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.
// </copyright>

using System;
using System.Diagnostics.Metrics;
using System.Reflection;
using Diagnostics = System.Diagnostics;
Expand All @@ -24,31 +25,40 @@ internal class ProcessMetrics
{
internal static readonly AssemblyName AssemblyName = typeof(ProcessMetrics).Assembly.GetName();
internal static readonly Meter MeterInstance = new(AssemblyName.Name, AssemblyName.Version.ToString());
private static readonly Diagnostics.Process CurrentProcess = Diagnostics.Process.GetCurrentProcess();

static ProcessMetrics()
{
// Refer to the spec for instrument details:
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/process-metrics.md#metric-instruments

InstrumentsValues values = new InstrumentsValues(() => (-1, -1, -1, -1));

// TODO: change to ObservableUpDownCounter
MeterInstance.CreateObservableGauge(
"process.memory.usage",
() =>
{
CurrentProcess.Refresh();
return CurrentProcess.WorkingSet64;
},
() => values.GetMemoryUsage(),
unit: "By",
description: "The amount of physical memory in use.");
description: "The amount of physical memory in use by the current process.");

// TODO: change to ObservableUpDownCounter
MeterInstance.CreateObservableGauge(
"process.memory.virtual",
() =>
{
CurrentProcess.Refresh();
return CurrentProcess.VirtualMemorySize64;
},
() => values.GetVirtualMemoryUsage(),
unit: "By",
description: "The amount of committed virtual memory.");
description: "The amount of committed virtual memory by the current process.");

MeterInstance.CreateObservableCounter(
$"process.cpu.time",
() => values.GetCpuTime(),
unit: "s",
description: "The amount of time that the current process has actively used a CPU to perform computations.");
Copy link

Choose a reason for hiding this comment

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

would be good to specify the measurement time unit in the description


// TODO: change to ObservableUpDownCounter
MeterInstance.CreateObservableGauge(
$"process.cpu.utilization",
() => values.GetCpuUtilization(),
unit: "s",
description: "Difference in process.cpu.time since the last collection of observerble instruments from the MetricReader, divided by the elapsed time multiply by the number of CPUs available to the current process.");
Copy link

Choose a reason for hiding this comment

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

CPU utilization percentage since last collection would be better?

The unit should be a percentage here?

}

/// <summary>
Expand All @@ -58,4 +68,86 @@ static ProcessMetrics()
public ProcessMetrics(ProcessInstrumentationOptions options)
{
}

private class InstrumentsValues
{
private const int TicksPerSecond = 10 * 7;
Copy link

@tarekgh tarekgh Sep 9, 2022

Choose a reason for hiding this comment

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

private readonly Diagnostics.Process currentProcess = Diagnostics.Process.GetCurrentProcess();

private double? memoryUsage;
private double? virtualMemoryUsage;
private double? cpuTime;
private double? cpuUtlization;

private DateTime lastCollectionTimestamp = DateTime.Now;
Copy link

Choose a reason for hiding this comment

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

Use DateTime.UtcNow instead of DateTime.Now. It is much faster.

Copy link
Member

Choose a reason for hiding this comment

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

In addition, these are not monotonic, I think for duration we should use something that measures elapsed time rather than wall clock.

private double lastCollectionCpuTime;

public InstrumentsValues(Func<(double MemoryUsage, double VirtualMemoryUsage, double CpuTime, double CpuUtilization)> readValues)
{
this.memoryUsage = null;
this.virtualMemoryUsage = null;
this.cpuTime = null;
this.cpuUtlization = null;

this.currentProcess.Refresh();
this.UpdateValues = readValues;
}

private Func<(double MemoryUsage, double VirtualMemoryUsage, double CpuTime, double CpuUtilization)> UpdateValues { get; set; }

public double GetMemoryUsage()
{
if (!this.memoryUsage.HasValue)
{
(this.memoryUsage, this.virtualMemoryUsage, this.cpuTime, this.cpuUtlization) = this.UpdateValues();
}

var value = this.currentProcess.WorkingSet64;
this.memoryUsage = null;
return value;
}

public double GetVirtualMemoryUsage()
{
if (!this.virtualMemoryUsage.HasValue)
{
(this.memoryUsage, this.virtualMemoryUsage, this.cpuTime, this.cpuUtlization) = this.UpdateValues();
}

var value = this.currentProcess.VirtualMemorySize64;
this.virtualMemoryUsage = null;
return value;
}

public double GetCpuTime()
{
if (!this.cpuTime.HasValue)
{
(this.memoryUsage, this.virtualMemoryUsage, this.cpuTime, this.cpuUtlization) = this.UpdateValues();
}

var value = this.currentProcess.TotalProcessorTime.Ticks * TicksPerSecond;
this.cpuTime = null;
return value;
}

public double GetCpuUtilization()
{
if (!this.cpuUtlization.HasValue)
{
(this.memoryUsage, this.virtualMemoryUsage, this.cpuTime, this.cpuUtlization) = this.UpdateValues();
}

var elapsedTime = (DateTime.Now - this.lastCollectionTimestamp).Ticks * TicksPerSecond;
Copy link

Choose a reason for hiding this comment

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

To convert ticks to seconds, you need to divide the ticks by the TicksPerSecond.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, this is embarrassing, thanks for pointing it out.

var currentCpuTime = this.currentProcess.TotalProcessorTime.Ticks * TicksPerSecond;
var deltaInCpuTime = currentCpuTime - this.lastCollectionCpuTime;

this.lastCollectionTimestamp = DateTime.Now;
Copy link

Choose a reason for hiding this comment

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

This will be DateTime.UtcNow too.

this.lastCollectionCpuTime = currentCpuTime;

var value = deltaInCpuTime / (Environment.ProcessorCount * elapsedTime);
this.cpuUtlization = null;
return value;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ public void ProcessMetricsAreCaptured()

meterProvider.ForceFlush(MaxTimeToAllowForFlush);

Assert.True(exportedItems.Count == 2);
Assert.True(exportedItems.Count == 4);
var physicalMemoryMetric = exportedItems.FirstOrDefault(i => i.Name == "process.memory.usage");
Assert.NotNull(physicalMemoryMetric);
var virtualMemoryMetric = exportedItems.FirstOrDefault(i => i.Name == "process.memory.virtual");
Assert.NotNull(virtualMemoryMetric);
var cpuTimeMetric = exportedItems.FirstOrDefault(i => i.Name == "process.cpu.time");
Assert.NotNull(cpuTimeMetric);
var cpuUtilizationMetric = exportedItems.FirstOrDefault(i => i.Name == "process.cpu.utilization");
Assert.NotNull(cpuUtilizationMetric);
}
}