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

Use commit SHA as additional version fallback #2664

Open
jamescrosswell opened this issue Sep 26, 2023 · 13 comments
Open

Use commit SHA as additional version fallback #2664

jamescrosswell opened this issue Sep 26, 2023 · 13 comments
Milestone

Comments

@jamescrosswell
Copy link
Collaborator

// Note: even though the informational version could be "invalid" (e.g. string.Empty), it should
// be used for versioning and the software should not fallback to the assembly version string.
// See https://github.com/getsentry/sentry-dotnet/pull/1079#issuecomment-866992216
// TODO: Lets change this in a new major to return the Version as fallback
return assembly.GetName().Version?.ToString();

@jamescrosswell jamescrosswell added this to the 4.0.0 milestone Sep 26, 2023
@bruno-garcia
Copy link
Member

bruno-garcia commented Sep 26, 2023

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
Copy link
Member

bruno-garcia commented Oct 6, 2023

Also another thing we can do on 4.0: getsentry/sentry-javascript#9195 (comment)

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
Copy link
Member

bruno-garcia commented Oct 6, 2023

@jamescrosswell
Copy link
Collaborator Author

jamescrosswell commented Oct 31, 2023

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:

var informationalVersion = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
if (!string.IsNullOrEmpty(informationalVersion))
{
return informationalVersion;
}

I think we can close this ticket then?

@jamescrosswell jamescrosswell changed the title Use Version as fallback Use commit SHA as additional version fallback Oct 31, 2023
@bruno-garcia
Copy link
Member

bruno-garcia commented Nov 5, 2023

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:

<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />

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

[email protected]+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
Copy link
Member

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

@jamescrosswell
Copy link
Collaborator Author

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
Copy link
Member

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
Copy link
Member

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 bruno-garcia modified the milestones: 4.0.0, 5.0.0 Nov 6, 2023
@bruno-garcia
Copy link
Member

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

@jamescrosswell
Copy link
Collaborator Author

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

@jamescrosswell
Copy link
Collaborator Author

jamescrosswell commented Aug 6, 2024

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:

internal static string? GetVersion(this Assembly assembly)
{
try
{
var informationalVersion = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
if (!string.IsNullOrEmpty(informationalVersion))
{
return informationalVersion;
}
}
catch
{
// Note: on full .NET FX, checking the AssemblyInformationalVersionAttribute could throw an exception,
// therefore this method uses a try/catch to make sure this method always returns a value
}
return assembly.GetName().Version?.ToString();
}

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 bitsandfoxes removed this from the 5.0.0 milestone Aug 22, 2024
@bitsandfoxes
Copy link
Contributor

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

@bitsandfoxes bitsandfoxes added this to the 5.0.0 milestone Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants