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

[Instrumentation.Process] Added memory related metrics. #595

Merged
merged 19 commits into from
Aug 26, 2022
Merged

[Instrumentation.Process] Added memory related metrics. #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 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
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
729d4c8
refresh
Yun-Ting Aug 25, 2022
b731dff
update
Yun-Ting Aug 25, 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
23 changes: 23 additions & 0 deletions src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,39 @@

using System.Diagnostics.Metrics;
using System.Reflection;
using Diagnostics = System.Diagnostics;
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved

namespace OpenTelemetry.Instrumentation.Process;

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();
Copy link
Member

Choose a reason for hiding this comment

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

This, combined with Refresh, look like a source of race condition.

image

Copy link
Member

Choose a reason for hiding this comment

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

@Yun-Ting You could probably just use a closure a la:

    static ProcessMetrics()
    {
        Diagnostics.Process currentProcess = Diagnostics.Process.GetCurrentProcess();

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

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

Is Process.Refresh() expensive? We'll be invoking it twice here anytime metrics are collected. If it is expensive we might want to create a state object we can refresh once and then return the result to the two counters in the callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for pointing it out. Please refer to #612 for the update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please refer to the latest PR #624 instead. Thanks!


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

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

/// <summary>
Expand Down
24 changes: 24 additions & 0 deletions test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,32 @@
// limitations under the License.
// </copyright>

using System.Collections.Generic;
reyang marked this conversation as resolved.
Show resolved Hide resolved
using System.Linq;
using OpenTelemetry.Metrics;
using Xunit;

namespace OpenTelemetry.Instrumentation.Process.Tests;

public class ProcessMetricsTests
{
private const int MaxTimeToAllowForFlush = 10000;

[Fact]
public void ProcessMetricsAreCaptured()
{
var exportedItems = new List<Metric>();
using var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddProcessInstrumentation()
.AddInMemoryExporter(exportedItems)
.Build();

meterProvider.ForceFlush(MaxTimeToAllowForFlush);

Assert.True(exportedItems.Count == 2);
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);
}
}