AddResourceMonitoring() do no-operation if not supported OS

[juliankock]

Description

I'm currently working on MacOS and it seems like https://source.dot.net/#Microsoft.Extensions.Diagnostics.ResourceMonitoring/ResourceMonitoringServiceCollectionExtensions.cs,7ba4439b710f2460,references throws an exception if the OS is not supported.

I would prefer it did a no-op. We go through all OS' and we need to do:

if (!OperatingSystem.IsMacOS())
{
    builder.Services.AddResourceMonitoring();
}

In the calling code.

Reproduction Steps

Run builder.Services.AddResourceMonitoring();.

Expected behavior

Just do no-op and return early if not supported OS.

Actual behavior

Throws exception

Regression?

No response

Known Workarounds

Add if statement at consuming code

Configuration

No response

Other information

No response

[juliankock]

#5963

[evgenyfedorov2]

Can you please explain your use case in more details, please?
Otherwise, the proposed change would be misleading for users, the library does not indeed support MacOs

[juliankock]

Can you please explain your use case in more details, please? Otherwise, the proposed change would be misleading for users, the library does not indeed support MacOs

We have a team that uses various operating systems, including macOS and leveraging Aspire for local development to debug metrics. We do not containerize when doing local development, we use Aspires AddProject. We do this to be able to easily attach a debugger when running, instead of the container approach.

Without wrapping, builder.Services.AddResourceMonitoring(); we crash on MacOS hosts, whereas it works on Windows and Linux hosts.

I do believe the docs state clearly that the metrics are only supported by MacOS and Linux, therefore consumers should expect it not to appear as metrics.

Another approach would be to add to the docs that it throws a PlatformNotSupportedException in the XML comment, to clearly state that it is thrown.

However, running on a non-Windows or Linux host is quite common when developing .NET, so I wouldn't say it's exception worthy.

EDIT: spelling

[evgenyfedorov2]

I do believe the docs state clearly that the metrics are only supported by MacOS and Linux, therefore consumers should expect it not to appear as metrics.

Did you mean Windows and Linux?

However, running on a non-Windows or Linux host is quite common when developing .NET, so I wouldn't say it's exception worthy.

This exception is about being explicit and return early.

Another approach would be to add to the docs that it throws a PlatformNotSupportedException in the XML comment, to clearly state that it is thrown.

For now, can we go this route instead, please?

[evgenyfedorov2]

I sometimes develop on a MacOS as well and suffer from not being able to run unit tests for this library, even though most of them are mock-based, e.g. do not technically require a specific OS.

Having said that, if you have time, feel free to propose and, possibly, implement another change, which would include removal of the PlatformNotSupportException on MacOs and unit test refactoring (where necessary) so that as many tests as possible would run on MacOS. Otherwise, I will try do it myself sooner or later

[juliankock]

I do believe the docs state clearly that the metrics are only supported by MacOS and Linux, therefore consumers should expect it not to appear as metrics.

Did you mean Windows and Linux?

However, running on a non-Windows or Linux host is quite common when developing .NET, so I wouldn't say it's exception worthy.

This exception is about being explicit and return early.

Another approach would be to add to the docs that it throws a PlatformNotSupportedException in the XML comment, to clearly state that it is thrown.

For now, can we go this route instead, please?

My fault, I of course mean Windows and Linux.

I sometimes develop on a MacOS as well and suffer from not being able to run unit tests for this library, even though most of them are mock-based, e.g. do not technically require a specific OS.

Having said that, if you have time, feel free to propose and, possibly, implement another change, which would include removal of the PlatformNotSupportException on MacOs and unit test refactoring (where necessary) so that as many tests as possible would run on MacOS. Otherwise, I will try do it myself sooner or later

I will see if I have time during the week to finish it up the implementation and the tests.

If not, I think a good start would be to change it to document it throws a PlatformNotSupportException on MacOS.