pack profiling nuget package

[bruno-garcia]

Testing bundling of the deps with in the profiling package

• Updates Microsoft.Diagnostics.NETCore.Client to 0.2.452401
• Adds target net8.0

It works (as in, sends profiles to Sentry) but I get some long unknown instead of the functions I have spinning (the prime number examples from our samples):

It's kind of fragile right now because the Sentry.Profiling project has a dependency on the perfview project. And then it manually adds 2 DLLs to the final package. I tested this by adding the nuget package via PackageReference to a test app and it worked fine. But ideally we'd have the other solution build outside of this project and only refer to the DLLs to get a behavior closer to what the consumers of the package will get.

But this could unblock packaging nonetheless.

In the future, if this package turns out to work out well enough, we can put more effort into this, including moving it all under Sentry and dropping Sentry.Profiling altogether.

[github-actions]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 5f9c4ef

[vaind]

LGTM, thanks

edit: maybe add this to .craft.yml in the same PR so it doesn't get forgotten?

[vaind]

Out of curiosity - why do we need to add net8 here?

[bruno-garcia]

The only 'strong' reason I can defend here is so it in this case is that if this package is added alone, it'll bring Sentry as a transient dep. If we don't have net8.0 here, it'll pull net6.0 from Sentry.

But wrt these decisions the mainstream idea is: In libs keep the lowest you need. But in reality there are some other cases where you want to bump (e.g ns2.0 is supported by net461 but causes issues due to rep resolution so years ago we had to ad net461 everywhere).

Recently on the .NET Discord someone from the .NET team was arguing against just saying "just keep the lowest" because we miss out on compiler improvements and other things added by the latest SDK when compiling to a newer target. Their take is to always support the latest version. Of course that has a cost so we can take that as a grain of salt.

This package specifically is something I hope we'll heavy invest, and will unblock new use cases for .NET folks using Sentry for performance monitoring. And since this is hopefully aligning with the core Sentry package eventually (as in, merging into a single page) having this already on net8.0 made sense to me.

[vaind]

Thanks for the thorough explanation, makes sense.

[vaind]

It's kind of fragile right now because the Sentry.Profiling project has a dependency on the perfview project. And then it manually adds 2 DLLs to the final package. I tested this by adding the nuget package via PackageReference to a test app and it worked fine. But ideally we'd have the other solution build outside of this project and only refer to the DLLs to get a behavior closer to what the consumers of the package will get.

We can test the final package like this: https://github.com/getsentry/sentry-dotnet/blob/feat/4.0.0/integration-test/runtime.Tests.ps1

[bruno-garcia]

It's kind of fragile right now because the Sentry.Profiling project has a dependency on the perfview project. And then it manually adds 2 DLLs to the final package. I tested this by adding the nuget package via PackageReference to a test app and it worked fine. But ideally we'd have the other solution build outside of this project and only refer to the DLLs to get a behavior closer to what the consumers of the package will get.

We can test the final package like this: feat/4.0.0/integration-test/runtime.Tests.ps1

Cool. I tested locally for now so happy to merge if ya'll are OK with the change

[bruno-garcia]

I'll keep it out for now since versioning is in tandem with Sentry and it requires pushing a new config in release registry.
So we don't really need it in the registry