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 Cpu related metrics and addressed comments. #612

Closed
wants to merge 40 commits into from
Closed
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
1a4d561
initial
Yun-Ting Aug 19, 2022
654601a
AssemblyInfo
Yun-Ting Aug 19, 2022
512dd21
Merge branch 'main' into yunl/pInst2
Yun-Ting Aug 19, 2022
23a6d35
nit
Yun-Ting Aug 19, 2022
dedd949
nit
Yun-Ting Aug 19, 2022
aa9a818
CI
Yun-Ting Aug 19, 2022
8a3e283
merge main
Yun-Ting Aug 23, 2022
6a95d5b
initial
Yun-Ting Aug 23, 2022
58f9dac
update
Yun-Ting Aug 24, 2022
0b111bb
Merge branch 'main' into yunl/pInst2
Yun-Ting Aug 24, 2022
a1279b2
adding a new metric
Yun-Ting Aug 24, 2022
db31dac
convention
Yun-Ting Aug 24, 2022
6bdd043
Merge branch 'yunl/pInst2' of https://github.com/Yun-Ting/opentelemet…
Yun-Ting Aug 25, 2022
05d4478
semantics
Yun-Ting Aug 25, 2022
f1d267f
comments
Yun-Ting Aug 25, 2022
12e12ba
Merge branch 'main' into yunl/pInst2
Yun-Ting Aug 25, 2022
6f9d879
comment
Yun-Ting Aug 25, 2022
07450d5
comment
Yun-Ting Aug 25, 2022
1a63dc7
merge main
Yun-Ting Aug 25, 2022
b19a2f6
cpuTime
Yun-Ting Aug 26, 2022
d18d8c0
Merge branch 'main' into yunl/pInst4
cijothomas Aug 26, 2022
371dbe3
nit
Yun-Ting Aug 26, 2022
3a6c115
refresh once
Yun-Ting Aug 26, 2022
dc81215
ctor
Yun-Ting Aug 26, 2022
9c6d2b0
param
Yun-Ting Aug 26, 2022
57f2183
comments
Yun-Ting Aug 26, 2022
ef37cce
update
Yun-Ting Aug 26, 2022
550966c
sanity
Yun-Ting Aug 26, 2022
879bb9d
Oops
Yun-Ting Aug 26, 2022
35c6186
comment
Yun-Ting Aug 26, 2022
ce98e93
unused
Yun-Ting Aug 26, 2022
8e38272
update
Yun-Ting Aug 26, 2022
10b5773
removed state option
Yun-Ting Aug 29, 2022
893fbac
api
Yun-Ting Aug 29, 2022
553efa4
added cpuUtil
Yun-Ting Aug 29, 2022
a24055f
update
Yun-Ting Sep 7, 2022
7e9198b
Merge branch 'main' into yunl/pInst4
Yun-Ting Sep 7, 2022
e498b06
update
Yun-Ting Sep 7, 2022
5167722
Merge branch 'yunl/pInst4' of https://github.com/Yun-Ting/opentelemet…
Yun-Ting Sep 7, 2022
e3fd84a
fix refresh
Yun-Ting Sep 7, 2022
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
OpenTelemetry.Instrumentation.Process.ProcessInstrumentationOptions
OpenTelemetry.Instrumentation.Process.ProcessInstrumentationOptions.CpuStatesEnabled.get -> bool?
OpenTelemetry.Instrumentation.Process.ProcessInstrumentationOptions.CpuStatesEnabled.set -> void
OpenTelemetry.Instrumentation.Process.ProcessInstrumentationOptions.ProcessInstrumentationOptions() -> void
OpenTelemetry.Metrics.MeterProviderBuilderExtensions
static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddProcessInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.Process.ProcessInstrumentationOptions> configure = null) -> OpenTelemetry.Metrics.MeterProviderBuilder
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,9 @@ namespace OpenTelemetry.Instrumentation.Process;
/// </summary>
public class ProcessInstrumentationOptions
{
/// <summary>
/// Gets or sets the flag indicating whether CPU time should be further broken down by its states.
/// The CPU state could be one of the following type: system, user, or wait.
/// </summary>
public bool? CpuStatesEnabled { get; set; } = false;
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved
}
89 changes: 72 additions & 17 deletions src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
// limitations under the License.
// </copyright>

using System;
using System.Collections.Generic;
using System.Diagnostics.Metrics;
using System.Reflection;
using Diagnostics = System.Diagnostics;
Expand All @@ -24,38 +26,91 @@ 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()
/// <summary>
/// Initializes a new instance of the <see cref="ProcessMetrics"/> class.
/// </summary>
/// <param name="options">The options to define the metrics.</param>
public ProcessMetrics(ProcessInstrumentationOptions? options)
{
InstrumentsValues values = new InstrumentsValues();

// 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.");

// 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.");

if (options.CpuStatesEnabled == false)
{
MeterInstance.CreateObservableCounter(
$"process.cpu.time",
() => values.GetProcessorCpuTime(),
unit: "s",
description: "Total CPU seconds.");
}
else
{
MeterInstance.CreateObservableCounter(
$"process.cpu.time",
() => values.GetProcessorCpuTimeWithBreakdown(),
unit: "s",
description: "Total CPU seconds broken down by different states.");
}
}

/// <summary>
/// Initializes a new instance of the <see cref="ProcessMetrics"/> class.
/// </summary>
/// <param name="options">The options to define the metrics.</param>
public ProcessMetrics(ProcessInstrumentationOptions options)
private class InstrumentsValues
{
private readonly Diagnostics.Process currentProcess = Diagnostics.Process.GetCurrentProcess();

public InstrumentsValues()
{
this.currentProcess.Refresh();
}

private enum CpuState
{
System,
User,
Wait,
}

public long GetMemoryUsage()
{
return this.currentProcess.WorkingSet64;
}

public long GetVirtualMemoryUsage()
{
return this.currentProcess.VirtualMemorySize64;
}

public long GetProcessorCpuTime()
{
return this.currentProcess.TotalProcessorTime.Seconds;
}

// See spec:
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/process-metrics.md#metric-instruments
public Measurement<long>[] GetProcessorCpuTimeWithBreakdown()
{
Measurement<long>[] measurements = new Measurement<long>[Enum.GetNames(typeof(CpuState)).Length];
var priviledgedCpuTime = this.currentProcess.PrivilegedProcessorTime.Seconds;
var userCpuTime = this.currentProcess.UserProcessorTime.Seconds;
Copy link
Member

Choose a reason for hiding this comment

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

Do we want integer or double?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use int.

Copy link
Member

Choose a reason for hiding this comment

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

Why int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, because the .Seconds from the Timespan struct returns int?

Copy link
Member

Choose a reason for hiding this comment

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

well, because the .Seconds from the Timespan struct returns int?

How is this related?

Choose a reason for hiding this comment

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

Shouldn't it be .TotalSeconds?


measurements[(int)CpuState.System] = new(priviledgedCpuTime, new KeyValuePair<string, object>("state", CpuState.System.ToString()));
reyang marked this conversation as resolved.
Show resolved Hide resolved
measurements[(int)CpuState.User] = new(userCpuTime, new KeyValuePair<string, object>("state", CpuState.User.ToString()));
measurements[(int)CpuState.Wait] = new(this.currentProcess.TotalProcessorTime.Seconds - priviledgedCpuTime - userCpuTime, new KeyValuePair<string, object>("state", CpuState.Wait.ToString()));
Copy link
Member

Choose a reason for hiding this comment

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

Will this give a non-zero value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The browser does not show me a very well-defined highlight for this comment. May I ask which part do you refer to?

Copy link
Member

Choose a reason for hiding this comment

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

What does TotalProcessorTime - PriviledgedCpuTime - UserCpuTime actually mean?

Copy link
Member

Choose a reason for hiding this comment

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

If you read this https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.process.totalprocessortime?view=net-6.0#property-value:

TotalProcessorTime - PriviledgedCpuTime - UserCpuTim is always going to give zero, unless .NET or the underlying native API has bugs.

image


return measurements;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ public void ProcessMetricsAreCaptured()

meterProvider.ForceFlush(MaxTimeToAllowForFlush);

Assert.True(exportedItems.Count == 2);
Assert.True(exportedItems.Count == 3);
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 processCpuTimeMetric = exportedItems.FirstOrDefault(i => i.Name == "process.cpu.time");
Assert.NotNull(processCpuTimeMetric);
}
}