PerformanceCounterLib calls RegistryKey.OpenRemoteBaseKey with incorrect machine name

[benpye]

Hey all - I think there is a bug with opening performance counters on remote machines. I have noticed that we have different behaviour to PDH, and I'm fairly certain theirs is correct.

Whilst the documentation for OpenRemoteBaseKey doesn't say much about the format of machineName, if we look at the docs for RegConnectRegistryW we see

The name of the remote computer. The string has the following form:

\computername

I do suspect that even this is a typo and it should be \\computername but the second \ has been eaten as an escape.

If we look at PerformanceCounterLib we do the right thing in one place https://github.com/dotnet/runtime/blob/master/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs#L678 , but the other two calls https://github.com/dotnet/runtime/blob/master/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs#L1044 and https://github.com/dotnet/runtime/blob/master/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs#L1286 seem to be doing the wrong thing. This bug also appears to exist in .NET Framework - and actually that's where I first noticed this issue.

This appears to make it impossible, at least on my machine, to use the .NET perf counter library against a remote machine. Unfortunately we can't pass in the correct machine name as we 'validate' the machine name and ensure it does not contain any \ characters! So far as I can tell this should be an easy fix, but I am surprised that it has gone undetected for so long.

[Dotnet-GitSync-Bot]

I couldn't add an area label to this Issue.

Checkout this page to find out which area owner to ping, or please add exactly one area label to help train me in the future.

[benpye]

@Anipik it looks like you own this?

[danmoseley]

if we look at the docs for RegConnectRegistryW we see

You probably know this, but note that you can click the pencil in the top right of the topic and fix it if you like.

[Anipik]

i will take a look

[benpye]

Adding a simple example

Currently the following will fail:
var categories = PerformanceCounterCategory.GetCategories("192.168.1.42");

I would expect to receive the list of performance counter categories from the remote machine. Instead I am getting an exception.

[danmoseley]

@benpye any interest in offering a PR, with tests (perhaps against 127.0.0.1?)?

[benpye]

@danmosemsft How do we deal with tests that require machine setup? To test this functionality we must go and enable the remote registry service on the target.

[danmoseley]

@benpye our options are

1. Add something like [Fact(Skip = "Manual test")] and run it locally only. That's possibly better than no test at all, as someone can comment that line and run it if they change the code in future.
2. Find something that would exercise the codepath but still succeed against the local box,. eg 127.0.0.1 \\%machinename% or something of that sort.
3. Forever maintain some endpoint for test use. This is what some networking tests do.

I don't think we would do 3 for this. 2 would be nice.

[benpye]

I am happy to add a test with 2 - however it looks like I can't repro the bug against localhost, but I can at least add a test that ensures that case doesn't regress. I guess it's the interaction between impersonation and OpenRemoteBaseKey that's causing trouble here.