getsentry / sentry-dotnet

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

Use commit SHA as additional version fallback #2664

Open jamescrosswell opened 1 year ago

jamescrosswell commented 1 year ago

https://github.com/getsentry/sentry-dotnet/blob/e75d537c14ded617c99eb33f19cf4ce69ca4de36/src/Sentry/Reflection/AssemblyExtensions.cs#L44-L48

bruno-garcia commented 1 year ago

Also please note there is an initative to have a default release in all SDKs, based on running git at compile time. @smeubank might know more about that

bruno-garcia commented 12 months ago

Also another thing we can do on 4.0: https://github.com/getsentry/sentry-javascript/discussions/9195#discussioncomment-7211748

Lets have more fallbacks where we get the SHA from one of those env vars at compile time? We have something that at compile timer adds an [assembly: .. attribute already I believe.

That is, unless newest versions of .NET have the AssemblyInformationVersion populated automatically, in which case we'd be reinventing the wheel

bruno-garcia commented 12 months ago

Oh @AbhiPrasad found it:

jamescrosswell commented 11 months ago

That is, unless newest versions of .NET have the AssemblyInformationVersion populated automatically, in which case we'd be reinventing the wheel

Whatever people put in the AssemblyInformationalVersionAttribute can be obtained at run time from the Application.ProductVersion property.

Typically, a version number displays as major number.minor number.build number.private part number. You can set it explicitly by setting the assembly version within your assembly manifest.

Folks can add a SHA in the private part number by passing /p:SourceRevisionId=foo when building the assembly (Hanselman describes this). But if they're doing that then we're already capturing that here right: https://github.com/getsentry/sentry-dotnet/blob/3d8bb0aad6ef86712ab49377e636a9c223028a87/src/Sentry/Reflection/AssemblyExtensions.cs#L32-L36

I think we can close this ticket then?

bruno-garcia commented 11 months ago

Whatever people put in the AssemblyInformationalVersionAttribute can be obtained at run time from the Application.ProductVersion property.

That's a .NET Framework API. Not available on .NET (Core)

Folks can add a SHA in the private part number by passing /p:SourceRevisionId=foo when building the assembly (Hanselman describes this).

He describes the parameter but you need to pass the actual value as an argument. The goal was for us to automate this (the same way that this does it for us:

https://github.com/getsentry/sentry-dotnet/blob/589ea4d3cc39b0277f874ffba49b2c7ca614f86d/src/Directory.Build.props#L56

If they are using this lib ^ or another SourceLink library as such it would already be setting AssemblyInformationalVersionAttribute properly and we wouldn't need to do anything.

For example, NuGet Trends has: https://github.com/dotnet/nuget-trends/blob/b4fbc342fec8159e01565a2dd8ae0e7362245338/src/Directory.Build.props#L20 Version isn't set in any other way when configuring Sentry (that I recall) and here's how releases show up in Sentry:

image

NuGetTrends.Web@1.0.0+b4fbc342fec8159e01565a2dd8ae0e7362245338

so: AssemblyName@AssemblyVersion+GitSHA

AssemblyVersion with 3 digits instead of the default 4, semver compatible. This is format is ideal. If I'm not mistaken we're getting this from our logic of concatenating the assemblyname + @ + whatever we get from InformationalVersion that's set by that SourceLink package at build time.

bruno-garcia commented 11 months ago

If we decide to change the release building logic, 4.0.0 is ideal time for "breaking" changes

jamescrosswell commented 11 months ago

The goal was for us to automate this (the same way that this does it for us:

I'm not sure I follow. So if someone is not using something like SourceLink already, we want a mechanism in Sentry for them to able to set the private build number portion of their assembly version to a commit hash when building their application? Or we simply want to read this?

I'm assuming it's not about setting this since dotnet build already allows you to pass a SourceRevisionId property to do this.

In terms of reading it, we'd just need to pull this off the attribute that gets set. This code is a bit hacked together, but will do it:

using System.Reflection;

// Get the assembly for the current executable
var assembly = Assembly.GetExecutingAssembly();

// Get the AssemblyInformationalVersionAttribute 
var infoVersion = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>();

// Extract just the informational version part
var privateBuildNumber = infoVersion?.InformationalVersion.Split('+')[1];

Console.WriteLine(privateBuildNumber); 
bruno-garcia commented 11 months ago

I'm not sure I follow. So if someone is not using something like SourceLink already, we want a mechanism in Sentry for them to able to set the private build number portion of their assembly version to a commit hash when building their application? Or we simply want to read this?

We would keep the value in a Sentry-specific "GitSha" attribute, so we could read it at runtime. That's only useful if they don't have set Release programatically. Nor have set AssemblyVersion (defaults to 1.0.0) so at least the commit sha differs. And if they don't use SourceLink.

I'm assuming it's not about setting this since dotnet build already allows you to pass a SourceRevisionId property to do this.

It's not about hoping the user does something manually (like passing the value). It's about Sentry working OOTB with no user configuration required. We'll show commit sha in the version (appended after version number through + as shown above).

In terms of reading it, we'd just need to pull this off the attribute that gets set. This code is a bit hacked together, but will do it:

Something like that but wouldn't be AssemblyInformationalVersionAttribute that we already have. We'd read from our Sentry specific attribute.

bruno-garcia commented 11 months ago

I think if we used sometjhing like:

    // GitHub Actions - https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables
    process.env.GITHUB_SHA ||
    // Netlify - https://docs.netlify.com/configure-builds/environment-variables/#build-metadata
    process.env.COMMIT_REF ||
    // Vercel - https://vercel.com/docs/v2/build-step#system-environment-variables
    process.env.VERCEL_GIT_COMMIT_SHA ||
    process.env.VERCEL_GITHUB_COMMIT_SHA ||
    process.env.VERCEL_GITLAB_COMMIT_SHA ||
    process.env.VERCEL_BITBUCKET_COMMIT_SHA ||
    // Zeit (now known as Vercel)
    process.env.ZEIT_GITHUB_COMMIT_SHA ||
    process.env.ZEIT_GITLAB_COMMIT_SHA ||
    process.env.ZEIT_BITBUCKET_COMMIT_SHA ||

As they did in Node: minus some obvious stuff like VERCELL and instead whatever TeamCity, AppVayor, Azure DevOps and other popular .NET building tools this would be good enough

bruno-garcia commented 11 months ago

Moved to 5.0.0 since we have too much in scope right now

jamescrosswell commented 11 months ago

Ah I see... I was wondering where this could come from if the user wasn't supplying it. That makes sense 👍🏻

jamescrosswell commented 2 months ago

From the AssemblyInformationalVersionAttribute docs:

Starting with the .NET 8 SDK, commit information is included in this attribute. This behavior applies to projects that target any .NET version. For more information, see Source Link included in the .NET SDK.

Build time

At build time we create releases when uploading symbols etc. during builds. We resolve the release version from:

  1. The SENTRY_RELEASE env variable
  2. The AssemblyInformationalVersionAttribute (which would include commit info when building with the .net8 SDK or later)
  3. The assembly version
  4. The Sentry-Cli proposed version (typically a commit hash)

So I think we're covered at build time.

Runtime

At runtime we're currently getting the version from the InformationalVersion attribute and then fall back to the assembly version:

https://github.com/getsentry/sentry-dotnet/blob/27ede88985fcc92ce2f7f8ae53ba11cfd805b2f5/src/Sentry/Reflection/AssemblyExtensions.cs#L27-L44

We don't yet have any fallback to a commit hash, although as noted above, we get this for free in the AssemblyInformationalVersionAttribute if people are building their applications (targeting any version of .NET) using version 8 or later of the .NET SDK.

Potential gaps

If people are building their .NET apps using the .NET SDK v7 or earlier and they haven't set any version information manually, currently the version information that gets sent in Sentry envelopes won't be very useful. We could potentially try to discover some kind of commit hash in these scenarios (e.g. using the technique Bruno described above).

bitsandfoxes commented 1 month ago

I'm adding this back to the 5.0 milestone. Either to have it closed or the gaps filled.