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 Sentry CLI after build to upload symbols #2107

Merged
merged 12 commits into from
Jan 10, 2023
Merged

Use Sentry CLI after build to upload symbols #2107

merged 12 commits into from
Jan 10, 2023

Conversation

mattjohnsonpint
Copy link
Contributor

As part of #2074, this PR adds the following:

  • Download and bundle the Sentry CLI with the Sentry nuget package
  • Call the Sentry CLI to upload debug information files (symbols, etc.) after a successful build.

Currently requires setting the following environment variables manually before running the build.

  • SENTRY_AUTH_TOKEN
  • SENTRY_ORG
  • SENTRY_PROJECT
  • SENTRY_URL (if self-hosted)

See https://docs.sentry.io/product/cli/configuration/

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

It's odd to say, but that's some pretty msbuild code :)

src/Sentry/buildTransitive/Sentry.props Outdated Show resolved Hide resolved
@mattjohnsonpint
Copy link
Contributor Author

Note, the verification snapshots previously had .../ in file paths because the buildTransitive\Sentry.props file was only imported to Sentry.Tests.csproj, so the project path assembly metadata attribute wasn't being added to the other test projects. Now that it's imported for all projects, it works correctly and strips the paths as intended.

@mattjohnsonpint
Copy link
Contributor Author

Another verification failure was due to the way we were scrubbing attributes on the API verification tests. On CI, the path string was sometimes long enough that the attribute was printed on two lines and we only scrubbed one of them. Fixed.

@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review January 6, 2023 21:40
@mattjohnsonpint mattjohnsonpint marked this pull request as draft January 6, 2023 21:41
@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review January 6, 2023 23:14
@mattjohnsonpint
Copy link
Contributor Author

Added the file hash verifications, and also went ahead and put the i686 (32-bit) versions in there too. Nuget size is 43MB, which is manageable.

@mattjohnsonpint mattjohnsonpint marked this pull request as draft January 7, 2023 01:38
@mattjohnsonpint
Copy link
Contributor Author

Going to do some refactoring before completing this, as we'll want to separate the Sentry CLI setup from the symbol upload part, such that we can re-use it for other Sentry CLI tasks like creating releases, etc.

@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review January 9, 2023 20:25
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Generally seems good to go. Only a couple notes to clarify

src/Sentry/buildTransitive/Sentry.props Outdated Show resolved Hide resolved
src/Sentry/buildTransitive/Sentry.props Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants