dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.36k stars 4.74k forks source link

`RuntimeInformation.OSDescription` is really `KernelDescription` #83287

Closed richlander closed 1 year ago

richlander commented 1 year ago

I have frequently used RuntimeInformation. It is a great type and I'm glad we have it. At some point, I stopped using RuntimeInformation.OSDescription on Linux since it returns values that are not very useful.

That's for two reasons:

Let's run this program

using System.Runtime.InteropServices;

Console.WriteLine(RuntimeInformation.OSDescription);

Bare metal (Raspberry Pi OS):

$ dotnet run
Linux 5.15.84-v8+ #1613 SMP PREEMPT Thu Jan 5 12:03:08 GMT 2023
$ cat /etc/os-release | head -n 1
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"

WSL + Containers (Debian):

$ dotnet run
Linux 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023
$ cat /etc/os-release | head -n 1
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"

Cloud (Azure + Ubuntu):

$ dotnet run
Linux 5.15.0-1034-azure #41-Ubuntu SMP Fri Feb 10 19:59:55 UTC 2023
$ cat /etc/os-release | head -n 1
PRETTY_NAME="Ubuntu 22.04.2 LTS"

You can see which uname variant we use:

$ uname -srv
Linux 5.15.0-1034-azure #41-Ubuntu SMP Fri Feb 10 19:59:55 UTC 2023

Windows:

>dotnet run
Microsoft Windows 10.0.22621

macOS:

% dotnet run
Darwin 22.4.0 Darwin Kernel Version 22.4.0: Wed Feb 22 22:16:19 PST 2023; root:xnu-8796.100.763.505.1~1/RELEASE_ARM64_T8103

I'd argue that the Linux kernel information serves no general purpose since the kernel is not the most important data for programs to know. For example, if containers are logging information, the kernel version is likely not the most important datapoint. Userland is likely much more important.

I propose two changes:

The RuntimeInformation.OSDescription docs state the following:

Gets a string that describes the operating system on which the app is running.

I'd suggest that this is inaccurate. For containers, for example, Debian, Ubuntu, Alpine, and others all return the same value.

I made these changes quite some time ago, but they are telling:

https://github.com/dotnet/dotnet-docker/blob/9db972bdead45dd4b4b5ed108e030bea98abcdf9/samples/dotnetapp/Program.cs#L25-L44

dotnet-issue-labeler[bot] commented 1 year ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.

Issue Details
I have frequently used `RuntimeInformation`. It is a great type and I'm glad we have it. At some point, I stopped using `RuntimeInformation.OSDescription` on Linux since it returns values that are not very useful. That's for two reasons: - It reports kernel information. - For containers, the kernel is the host kernel. Let's run this program ```csharp using System.Runtime.InteropServices; Console.WriteLine(RuntimeInformation.OSDescription); ``` Bare metal (Raspberry Pi OS): ```bash $ dotnet run Linux 5.15.84-v8+ #1613 SMP PREEMPT Thu Jan 5 12:03:08 GMT 2023 $ cat /etc/os-release | head -n 1 PRETTY_NAME="Debian GNU/Linux 11 (bullseye)" ``` WSL + Containers (Debian): ```bash $ dotnet run Linux 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 $ cat /etc/os-release | head -n 1 PRETTY_NAME="Debian GNU/Linux 11 (bullseye)" ``` Cloud (Azure + Ubuntu): ```bash $ dotnet run Linux 5.15.0-1034-azure #41-Ubuntu SMP Fri Feb 10 19:59:55 UTC 2023 $ cat /etc/os-release | head -n 1 PRETTY_NAME="Ubuntu 22.04.2 LTS" ``` You can see which `uname` variant we use: ```bash $ uname -srv Linux 5.15.0-1034-azure #41-Ubuntu SMP Fri Feb 10 19:59:55 UTC 2023 ``` Windows: ```bash >dotnet run Microsoft Windows 10.0.22621 ``` macOS: ```bash % dotnet run Darwin 22.4.0 Darwin Kernel Version 22.4.0: Wed Feb 22 22:16:19 PST 2023; root:xnu-8796.100.763.505.1~1/RELEASE_ARM64_T8103 ``` I'd argue that the Linux kernel information serves no general purpose since the kernel is not the most important data for programs to know. For example, if containers are logging information, the kernel version is likely not the most important datapoint. Userland is likely much more important. I propose two changes: - Change `OSDescription` to report information about the userland OS, which would mean rely on `/etc/os-release` on Linux (if present). - Consider adding a `KernelDescription` property to report the currently provided kernel information. The [`RuntimeInformation.OSDescription`](https://learn.microsoft.com/dotnet/api/system.runtime.interopservices.runtimeinformation.osdescription) docs state the following: > Gets a string that describes the operating system on which the app is running. I'd suggest that this is inaccurate. For containers, for example, Debian, Ubuntu, Alpine, and others all return the same value. I made these changes quite some time ago, but they are telling: https://github.com/dotnet/dotnet-docker/blob/9db972bdead45dd4b4b5ed108e030bea98abcdf9/samples/dotnetapp/Program.cs#L25-L44
Author: richlander
Assignees: -
Labels: `area-System.Runtime`, `untriaged`
Milestone: -
jkotas commented 1 year ago

Related / duplicate: https://github.com/dotnet/runtime/issues/37923

richlander commented 1 year ago

It's not too late @am11! I'll still be your PM!

richlander commented 1 year ago

Upon a little more thought, it seems like there are two choices here:

am11 commented 1 year ago

There is no standard way of describing an operating system and depending on the audience / use-case, any description will fall short.

Linux Standard Base (LSB) distros provide /etc/lsb-release ASCII file and a related utility that can be called as: lsb_release --all However, not all distros use LSB.

systemd introduced /etc/os-release ASCII file. Since not all distros embrace systemd, they may not provide this file. Those which do, values (of even the known keys) are unreliable across wider range of distros.

Here is a scary (and definitely incomplete) heuristic to identify a unix-y userland: https://github.com/offscale/osbuild/blob/4f8c3b02766cc3c163cd6b157a938931b89a24d1/src/distribution.c#L31-L116.

https://github.com/dotnet/runtime/issues/37923 was proposing to include this name (renamed from PAL_UNIX_NAME): https://github.com/dotnet/runtime/blob/8599161c16cbeb9ceee8a043038edff37400be5c/src/libraries/System.Private.CoreLib/src/System/OperatingSystem.cs#L12-L42 in OSDescription in some manner (preferably structured string appended to OS description; PlatformName: {OSPlatformName}), since that's how .NET Runtime libraries identify the current platform.

richlander commented 1 year ago

One option is to prefer /etc/os-release and then fallback to uname. That's probably a bad idea, since it would be hard to know which one you got.

In retrospect, it isn't obvious to me that this API is all that useful, for the reasons that you describe. I almost wonder if we should deprecate this API. It isn't that the data isn't useful, but that it is misleading. For containers and VMs, it is a lie. It seems like the higher-level functionality (if people want it) should be moved to a NuGet package.

danmoseley commented 1 year ago

@tmds do you have thoughts here?

tmds commented 1 year ago

I think the intended use-case for this API is to provide a descriptive string for the OS.

On Linux, returning something like /etc/os-release PRETTY_NAME (with fallbacks) would be the most descriptive.

Changing the description may break someone who is trying to extract information from it.

The preferred API to find if the OS is Linux is OperatingSystem.IsLinux() (or RuntimeInformation.IsOSPlatform). The preferred API to find the Linux kernel version is Environment.OSVersion.Version.

MichalPetryka commented 1 year ago

Maybe an api that is something like bool TryGetOsPrettyName(out string) should be exposed here? On linux it'd check the 2 files in etc and on Windows it could map the versions to names (ie 6.1 to 7 etc).

tmds commented 1 year ago

I prefer to change what the property returns, instead of adding something new.

tmds commented 1 year ago

I assume we're fine changing the description that is returned now?

Maybe we should make clear in the docs, this is a description and not meant for parsing.

In an earlier comment, I've mentioned the APIs to check for Linux and the Linux kernel version.

If you want to know the distro, RuntimeInformation.RuntimeIdentifier is what you can use.

stephentoub commented 1 year ago

I assume we're fine changing the description that is returned now?

Yes. Our delegation to another system to produce this means we're already subject to whatever changes the underlying system provides, as well.

we should make clear in the docs

Yes, that'd be good clarification to add. That's always been the case, but we should be explicit about it.

tmds commented 1 year ago

I think there is consensus that we're fine changing what is returned, and that we want a human friendly string.

I will look into updating the implementation so it returns $PRETTY_NAME from /etc/os-release when that is present. with a fallback to: $NAME[ $VERSION].

The os-release spec defines these fields as suitable for presentation to the user.

We can improve the logic further, as needed, by adding additional fallbacks in followup PRs.

Some /etc/os-release files:

NAME="Fedora Linux"
VERSION="37 (Workstation Edition)"
ID=fedora
VERSION_ID=37
VERSION_CODENAME=""
PLATFORM_ID="platform:f37"
PRETTY_NAME="Fedora Linux 37 (Workstation Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:37"
DEFAULT_HOSTNAME="fedora"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f37/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=37
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=37
SUPPORT_END=2023-11-14
VARIANT="Workstation Edition"
VARIANT_ID=workstation
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.17.2
PRETTY_NAME="Alpine Linux v3.17"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://gitlab.alpinelinux.org/alpine/aports/-/issues"
NAME="Arch Linux"
PRETTY_NAME="Arch Linux"
ID=arch
BUILD_ID=rolling
VERSION_ID=TEMPLATE_VERSION_ID
ANSI_COLOR="38;2;23;147;209"
HOME_URL="https://archlinux.org/"
DOCUMENTATION_URL="https://wiki.archlinux.org/"
SUPPORT_URL="https://bbs.archlinux.org/"
BUG_REPORT_URL="https://bugs.archlinux.org/"
PRIVACY_POLICY_URL="https://terms.archlinux.org/docs/privacy-policy/"
LOGO=archlinux-logo
tannergooding commented 1 year ago

@richlander, given this is effectively a dupe of https://github.com/dotnet/runtime/issues/37923 can we close this or the other so we have "one issue" to track any desired changes/improvements to OSDescription

Given we don't guarantee the value returned by OSDescription, changing it to provide more relevant information seems fine/generally good to me.

ghost commented 1 year ago

This issue has been marked needs-author-action and may be missing some important information.

tmds commented 1 year ago

@richlander, given this is effectively a dupe of https://github.com/dotnet/runtime/issues/37923 can we close this or the other so we have "one issue" to track any desired changes/improvements to OSDescription

I prefer to keep this issue because we settled on an implementation.

The other issue does have some extended API proposal which was not discussed here.

Anyway, I'll work on it shortly (next week, or the week after), so we'll be able to close both.

ghost commented 1 year ago

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

richlander commented 1 year ago

I finally tried this. It works! Thanks!