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

[UWP / WinUI] Operating system info missing in reports #3260

Open
tipa opened this issue Apr 4, 2024 · 14 comments
Open

[UWP / WinUI] Operating system info missing in reports #3260

tipa opened this issue Apr 4, 2024 · 14 comments
Labels
Feature New feature or request Windows

Comments

@tipa
Copy link

tipa commented Apr 4, 2024

Package

Sentry

.NET Flavor

.NET

.NET Version

8.0.2

OS

Windows

SDK Version

4.2.1

Self-Hosted Sentry Version

No response

Steps to Reproduce

Send test exception to Sentry from a UWP or WinUI3 app.

Expected Result

See detailed info about operating system - incl. build version, e.g. Windows 11 (build 22631.3374).

Actual Result

WinUI3:
image

  • The "os.name=Windows" doesn't give me any additional information compared to "os=Windows 11"
  • Build number missing (e.g. 22631.3374 or 10.0.22631)

UWP: no OS information at all

The build information can be gathered via Environment.OSVersion.Version. Additionally, it could be helpful to enrich the exception with the machine name (e.g. laptop model), similar how on iOS it shows "device=iPhone 13 Pro". This information can be gathered using Environment.MachineName

@jamescrosswell
Copy link
Collaborator

Thanks again @tipa!

It looks like we're currently only setting this for MAUI and Android:

@bitsandfoxes we could look at capturing a bit more detail, ideally across various different platforms. It's the kind of static information that we'd ideally populate once, lazily, at program startup.

@tipa
Copy link
Author

tipa commented Apr 8, 2024

It looks like we're currently only setting this for MAUI and Android:

The data also seems to be populated on iOS (I am not using MAUI, but .NET for iOS):
image

The information is also missing on macOS ("os.name" is always "Darwin", but os version is missing), could it be added for that platform as well?
image

@bitsandfoxes
Copy link
Contributor

I think that's super valuable feedback. We'd definitely like to provide information as relevant as possible. It might take a while to get around adding those. How do you feel about opening a PR adding those for your relevant platforms?

@tipa
Copy link
Author

tipa commented Apr 8, 2024

I'd be willing to help with this work and I had a closer look at the source code, but I was kind of overwhelmed by the complexity. I don't know if I would be able to provide a mergeable PR in reasonable time.
However, I did some more test of what information could be used:

macOS (.NET 8):
os: Environment.OSVersion.Version.ToString() returns 14.4.1 on a macOS Sonoma 14.4.1 machine
device: DeviceInfo.Model (see code below) returns Mac15,12 on a Macbook Air M3
https://github.com/xamarin/Essentials/blob/main/Xamarin.Essentials/DeviceInfo/DeviceInfo.macos.cs
https://github.com/xamarin/Essentials/blob/fcccb58a452bcd741680553182c09f8b9969c688/Xamarin.Essentials/Platform/Platform.macos.cs#L75

Windows (.NET 8 / WinUI 3 & UWP):
os: Environment.OSVersion.Version.ToString() returns 10.0.22631.0 on the latest Windows 11 release
device: new EasClientDeviceInformation().SystemProductName returns XPS 15 9500 on a Dell XPS 15 laptop
device.family: new EasClientDeviceInformation().SystemManufacturer returns Dell Inc. on a Dell XPS 15 laptop

@jamescrosswell
Copy link
Collaborator

@tipa we'd love the help! There's quite a bit going on right now so I suspect you'd be able to put something together before we'd have the bandwidth to prioritise this.

I'm happy to help with advice and reviews. As you gleaned, there's a bit of complexity to the codebase. Much of this comes from two things:

  1. We're building for all kinds of different platforms. Some people are running WinUI or WinForms on .NET Framework and others are using the SDK in Unity for games.
  2. We deliberately avoid adding any dependencies to Sentry where at all possible (especially the Sentry core package), since that's a quick road to dependency hell.

That means sometimes we have to get a bit creative about implementing solutions. We have to write stuff that takes advantage of the special features of framework XYZ, without requiring people running the software in other environments from having to add unecessary dependencies.

You might find it tricky to use EasClientDeviceInformation, for example - I think that would require a dependency on an external NuGet package. You'll see quite a bit of code in the SDK that's guarded by #if directives like #if WINDOWS etc. so we can write code that's Windows specific but probably need to use lower level APIs to get at information. I'd guess that, under the hood, EasClientDeviceInformation is getting it's information from WMI, which is probably just getting it from the Windows Registry. If we can find the registry key that this is being read from then we can probably just read it from there directly (and the APIs to read/write registry keys shouldn't have any dependency other than running on Windows). That can be a little bit challenging as different vendors aren't always consistent about how/where they store information in the Windows registry (Dell vs IBM vs HP etc.) but for really basic information, hopefully they're fairly well aligned.

@tipa
Copy link
Author

tipa commented Apr 10, 2024

Ok, I will see if I can wrap my head around the complexities and get some progress once I find some free time. Unfortunately, I have a very busy week as well

@tipa
Copy link
Author

tipa commented Apr 17, 2024

The information is also missing on macOS ("os.name" is always "Darwin", but os version is missing), could it be added for that platform as well?
image

image

@jamescrosswell Looking at the source code and Nuget - is macOS even really supported? I mean I can use it and it works (with the shortcomings that I reported in this and other issues) but I'd assume you would have to adjust the target frameworks when I wanted to add something AppKit-specific?

@jamescrosswell
Copy link
Collaborator

is macOS even really supported? I mean I can use it and it works (with the shortcomings that I reported in this and other issues) but I'd assume you would have to adjust the target frameworks when I wanted to add something AppKit-specific?

I assume we can get OS Information on macOS... it should be possible to get this without appKit.

This isn't available in the OperatingSystem Class?

@tipa
Copy link
Author

tipa commented Apr 17, 2024

@jamescrosswell the OS version can also be queried using Environment.OSVersion.Version.ToString(), as I suggested above. But for I didn't find something similar for the device model, which is why I suggested this (and likely requires AppKit):

device: DeviceInfo.Model (see code below) returns Mac15,12 on a Macbook Air M3 https://github.com/xamarin/Essentials/blob/main/Xamarin.Essentials/DeviceInfo/DeviceInfo.macos.cs https://github.com/xamarin/Essentials/blob/fcccb58a452bcd741680553182c09f8b9969c688/Xamarin.Essentials/Platform/Platform.macos.cs#L75

Also, I don't think I can use precompiler directives for macOS like this unless it is targeted explicitly:

#if MACOS
    // macOS specifics
#endif

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented Apr 17, 2024

Ah I see. Yes DeviceInfo is a bit trickier.

There are directives for compilation targeting Android, iOS and macCatalyst:

#if __ANDROID__
// TODO there will be support for NativeAOT in the future.
_nativeDebugImages ??= new();
#elif __IOS__ || MACCATALYST
_nativeDebugImages ??= Sentry.Cocoa.C.LoadDebugImages(_options.DiagnosticLogger);
#else
_nativeDebugImages ??= Sentry.Native.C.LoadDebugImages(_options.DiagnosticLogger);
#endif

But we don't have separate builds for different operating systems. Anything like that would have to be a runtime check, I think:

if ((_defaultIntegrations & DefaultIntegrations.WinUiUnhandledExceptionIntegration) != 0
&& RuntimeInformation.IsOSPlatform(OSPlatform.Windows))

I don't think we can get at AppKit directly though (only via the Sentry.Maui integration). MAUI essentially does all the kinds of stuff Xamarin used to do (provide .NET abstractions around platform specific libraries).

@tipa
Copy link
Author

tipa commented Apr 17, 2024

I don't want to use MAUI - it would pull in way too many dependencies just for the sake of getting device info...
How is it done for iOS? Device and other information is populated automatically and I don't see anything related in the .NET SDK. Is it set in the native Cocoa SDK? Could the same be done for macOS?
image

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented Apr 17, 2024

How is it done for iOS? Device and other information is populated automatically and I don't see anything related in the .NET SDK. Is it set in the native Cocoa SDK? Could the same be done for macOS?

I think it must be. In .NET we have code to pull it from Android:

And from MAUI:

device.Model ??= deviceInfo.Model;

But I don't see anything for iOS specifically.

The cocoa sdk definitely has code for this:
https://github.com/getsentry/sentry-cocoa/blob/38f4f70d07117b9f958a76b1bff278c2f29ffe0e/Sources/Sentry/SentryDevice.mm#L29

We can't do the same for macOS because we don't wrap any "native" SDK when building .NET apps for macOS. So the .NET runtime is what we have available on macOS.

The only exception to the above is when publishing applications with AOT compilation... in that case we wrap the sentry-native SDK for native crash reporting. Potentially when compiling AOT then, it would be possible to marshal some funtionality from sentry-native... that's not something I've dealt with yet.

@tipa
Copy link
Author

tipa commented Apr 17, 2024

I am using AOT compilation ("Native AOT") for both my iOS and macOS apps - so when there's a way to populate the device info for macOS using the native SDK, that would be much appreciated. Unforunately, this would be too complex for me to contribute in a PR

@jamescrosswell
Copy link
Collaborator

I am using AOT compilation ("Native AOT") for both my iOS and macOS apps - so when there's a way to populate the device info for macOS using the native SDK, that would be much appreciated. Unforunately, this would be too complex for me to contribute in a PR

Yeah fair enough... it sounds like scope creep 😁 I've created #3313 that we can look into separately. Maybe we just keep this PR focused on the OS Information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request Windows
Projects
Status: No status
Status: No status
Development

No branches or pull requests

3 participants