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.

Sign up for GitHub

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

Jump to bottom

pack profiling nuget package #2800

Merged
merged 5 commits into from
Nov 19, 2023
Merged

pack profiling nuget package #2800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e50b267
pack profiling
bruno-garcia Nov 6, 2023
5c493c6
Merge branch 'feat/4.0.0' into feat/profiling-pack
bruno-garcia Nov 19, 2023
f7ed3fa
wip
bruno-garcia Nov 19, 2023
0661bf6
wip
bruno-garcia Nov 19, 2023
5f9c4ef
wip
bruno-garcia Nov 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- `Sentry.Profiling` is now packaged to be uploaded to nuget.org ([#2800](https://github.com/getsentry/sentry-dotnet/pull/2800))

## 4.0.0-beta.0

### Fixes
Expand Down
23 changes: 18 additions & 5 deletions src/Sentry.Profiling/Sentry.Profiling.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@

<PropertyGroup>
<!-- TODO check and update the list of supported frameworks. -->
<TargetFrameworks>net6.0</TargetFrameworks>
<TargetFrameworks>net8.0;net6.0</TargetFrameworks>
Comment on lines 4 to +5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough explanation, makes sense.

<PackageTags>$(PackageTags);Profiling;Diagnostic</PackageTags>
<!-- TODO: re-enable packing once we've resolved all dependencies for Sentry.Profiling -->
<IsPackable>false</IsPackable>
<Description>Performance profiling support for Sentry - Open-source error tracking that helps developers monitor and fix crashes in real time.</Description>
</PropertyGroup>

Expand All @@ -15,13 +13,28 @@

<ItemGroup>
<ProjectReference Include="..\..\src\Sentry\Sentry.csproj" />
<PackageReference Include="Microsoft.Diagnostics.NETCore.Client" Version="0.2.421201" />
<ProjectReference Include="../../modules/perfview/src/TraceEvent/TraceEvent.csproj" />
<PackageReference Include="Microsoft.Diagnostics.NETCore.Client" Version="0.2.452401" />
<!-- This triggers the build of this project and its dependencies. We don't need all of them but this is the easiest way -->
<!-- to make sure the project builds/cleans etc in tandem with this. Packaging copies the 2 DLLs we need below -->
<ProjectReference Include="../../modules/perfview/src/TraceEvent/TraceEvent.csproj" PrivateAssets="all" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Sentry.Benchmarks" PublicKey="$(SentryPublicKey)" />
<InternalsVisibleTo Include="Sentry.Profiling.Tests" PublicKey="$(SentryPublicKey)" />
</ItemGroup>

<ItemGroup>
<ProfilingDependency Include="..\..\modules\perfview\src\FastSerialization\bin\$(Configuration)\netstandard2.0\Microsoft.Diagnostics.FastSerialization.dll" />
<ProfilingDependency Include="..\..\modules\perfview\src\TraceEvent\bin\$(Configuration)\netstandard2.0\Microsoft.Diagnostics.Tracing.TraceEvent.dll" />
<ProfilingDependency Include="..\..\modules\perfview\src\FastSerialization\bin\$(Configuration)\netstandard2.0\Microsoft.Diagnostics.FastSerialization.pdb" />
<ProfilingDependency Include="..\..\modules\perfview\src\TraceEvent\bin\$(Configuration)\netstandard2.0\Microsoft.Diagnostics.Tracing.TraceEvent.pdb" />
</ItemGroup>
<ItemGroup>
<!-- TODO: pdb's are getting packed on the nupkg (as well as the snupkg), figure out a way to exclude from nupkg-->
<TfmSpecificPackageFile Include="@(ProfilingDependency)">
<Pack>true</Pack>
<PackagePath>lib\$(TargetFramework)</PackagePath>
</TfmSpecificPackageFile>
</ItemGroup>
</Project>
8 changes: 8 additions & 0 deletions test/Sentry.Profiling.Tests/Sentry.Profiling.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,12 @@
<ProjectReference Include="..\Sentry.Testing\Sentry.Testing.csproj" />
</ItemGroup>

<ItemGroup>
<ProfilingDependency Include="..\..\modules\perfview\src\FastSerialization\bin\$(Configuration)\netstandard2.0\Microsoft.Diagnostics.FastSerialization.dll" />
<ProfilingDependency Include="..\..\modules\perfview\src\TraceEvent\bin\$(Configuration)\netstandard2.0\Microsoft.Diagnostics.Tracing.TraceEvent.dll" />
</ItemGroup>
<ItemGroup>
<Reference Include="@(ProfilingDependency)" />
</ItemGroup>

</Project>