search

getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
590 stars 206 forks source link

macOS version not reported #2710

Open bruno-garcia opened 1 year ago

bruno-garcia commented 1 year ago

example event: https://sentry-sdks.sentry.io/issues/4536804443/?project=5428537&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=1h&stream_index=1

image

Seems like macOS version has regressed

jamescrosswell commented 6 months ago

We are still collecting the OS version however it's stored in the Contexts (not in the Tags): https://github.com/getsentry/sentry-dotnet/blob/5bcb9cac8b54f283a4ce5cc16573bfdf2970cdf9/src/Sentry/Internal/Enricher.cs#L49

This is what I'm seeing come through to Sentry:

Operating System Name Darwin Kernel Version 23.4.0 Raw Description Darwin 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:12:49 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6020

It gives us the Darwin version, which is a bit obtuse. We could potentially try to pretty this up in the case of macOS by doing a bit of a lookup to work out the common name and version (e.g. Sonoma 14.4.1), but I believe we'd have to maintain that lookup table manually everytime Apple released a new macOS version.

bruno-garcia commented 6 months ago

Relay extract tags from the context. We shouldn't be sending any custom tag directly from the SDK. See: https://github.com/getsentry/relay/blob/24037f40afe892bf75bba3e74c4fc10c7f710bb1/relay-event-normalization/src/normalize/contexts.rs#L434

by doing a bit of a lookup to work out the common name and version (e.g. Sonoma 14.4.1), but I believe we'd have to maintain that lookup table manually everytime Apple released a new macOS version.

I wonder if this works for native Mac apps. Do we have mapping in Sentry (on Relay for that?) Or the SDK already sends the right value? @philipphofmann might know.

jamescrosswell commented 6 months ago

Relay extract tags from the context. We shouldn't be sending any custom tag directly from the SDK.

Ok, so currently the behaviour is slightly different for macOS than Windows.

On Windows you end up with tags for os and os.name:

image

On macOS you end up with just the os.name:

image

What is different on MacOS is that we provide the Kernel Version in the context, whereas on Windows it's Version:

image image

That's maybe what's tripping up the relay. I think it only looks for a value for os.version in the context.

Do we want to try to fix in the SDK or in the Relay? It might be nicer to do it in the relay, since otherwise we'd probably have to change multiple SDKs.

philipphofmann commented 6 months ago

MacOS version works on Cocoa: https://sentry-sdks.sentry.io/issues/5310027106/events/1b44a1146aab4fc98a547b815e22b8da/

CleanShot 2024-05-06 at 13 19 34

bitsandfoxes commented 6 months ago

Do we want to try to fix in the SDK or in the Relay?

This looks like a job that should be done in Relay.

bitsandfoxes commented 4 months ago

Reference for a similar thing for iOS and iPadOS for the Unity SDK. https://github.com/getsentry/relay/pull/3780

bruno-garcia commented 1 month ago

Would be nice to fix this. If we can do it in Relay, that's great. If we need to add netX-macos and get the proper OS name/version, we can do that too. The "Darwin" version isn't great.

@jamescrosswell you added blocked label, what do we need to unblock? Please raise a ticket on getsentry/relay if that will solve it

jamescrosswell commented 1 month ago

@jamescrosswell you added blocked label, what do we need to unblock? Please raise a ticket on getsentry/relay if that will solve it

Yeah it was blocked because I don't think we want to solve it in the sentry-dotnet repo. I've created a ticket in the relay repo now:

jamescrosswell commented 1 month ago

See @Dav1dde's reply:

Looks like we should try to get this from a different source. I'll dig around for a proper API to pull that information rather than trying to read it from system files (that we may not have read access to).

bruno-garcia commented 3 weeks ago

yeah I think we need to report the right data. We might need to add netX.0-macos if we need to #if MAC and access some native APIs.

Or actually we prob can get away with the P/Invoke signature for the function and just call the right thing if we're on a Mac. Like things used to be done before the platform specific TFMs.

Whatever they are calling in the cocoa SDK to get this data, we can call that via P/Invoke when running on a Mac