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

Improvements for Sentry CLI integration #2145

Merged
merged 9 commits into from
Jan 31, 2023
Merged

Improvements for Sentry CLI integration #2145

merged 9 commits into from
Jan 31, 2023

Conversation

mattjohnsonpint
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint commented Jan 31, 2023

  • Updates Sentry CLI to 2.12.0
  • Updates the msbuild target order to resolve On MAUI sentry-cli seems to only run before AOT, missing the AOT'd symbols #2137
  • Adds SentryUploadSources msbuild property. Set true to opt-in to uploading sources while uploading symbols. See Support for Source Context #646 (comment)
  • Now uploads from the project's bin folder by default, and adds a SentryIncludeIntermediateOutputPath property which can be set true to also include the obj folder.
  • Warns instead of erroring if upload fails.
  • If upload fails when SentryUploadSources is enabled, it will retry with symbols only (no sources).

@mattjohnsonpint
Copy link
Contributor Author

mattjohnsonpint commented Jan 31, 2023

We're getting build failures when sentry-cli upload-dif --include-sources is being called for Sentry.Samples.Maui.

I can repro locally as well. The error is:

> Found 1 debug information file
error: malformed debug info file
  caused by: invalid custom debug information table item tag 0

Adding --log-level=debug doesn't add any useful information.

@vaind @Swatinem - any ideas?

@mattjohnsonpint
Copy link
Contributor Author

Note, it doesn't seem to happen for net6.0-ios target of Sentry.Samples.Maui, nor does it happen for Sentry.Samples.Android.

The error is on that one project/target, and only goes away if I don't pass --include-sources.

@Swatinem
Copy link
Member

invalid custom debug information table item tag 0

That is very weird. 0 is supposed to be MethodDef, and no error should be raised in that case, unless of course the error happens here:

https://github.com/getsentry/symbolic/blob/f978ae6b004a8daa00798792ce70782442cda69c/symbolic-ppdb/src/format/mod.rs#L397

So we get an embedded source for a MethodDef? I think @vaind could take a closer look, he wrote that part of the code.

@vaind
Copy link
Collaborator

vaind commented Jan 31, 2023

I think @vaind could take a closer look, he wrote that part of the code.

Already on it

@vaind
Copy link
Collaborator

vaind commented Jan 31, 2023

We're getting build failures when sentry-cli upload-dif --include-sources is being called for Sentry.Samples.Maui.

Reproduced locally: fails on C:\dev\sentry-dotnet\tools\sentry-cli\2.12.0\sentry-cli-Windows-x86_64.exe upload-dif --include-sources --org sentry-sdks --project sentry-dotnet --log-level=debug obj\Release\net6.0-android\
More specifically on: Sentry.Samples.Maui.pdb and it turns out to be some auto-generated code:

[assembly: global::Microsoft.Maui.Controls.Xaml.XamlResourceId("Sentry.Samples.Maui.Resources.Styles.Colors.xaml", "Resources/Styles/Colors.xaml", typeof(global::__XamlGeneratedCode__.__Type5F181F850177F3A0))]
namespace __XamlGeneratedCode__
{
        [global::Microsoft.Maui.Controls.Xaml.XamlFilePath("Resources\\Styles\\Colors.xaml")]
        [global::Microsoft.Maui.Controls.Xaml.XamlCompilation(global::Microsoft.Maui.Controls.Xaml.XamlCompilationOptions.Compile)]
        [global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
        public partial class __Type5F181F850177F3A0 : global::Microsoft.Maui.Controls.ResourceDictionary
        {
                [global::System.CodeDom.Compiler.GeneratedCode("Microsoft.Maui.Controls.SourceGen", "1.0.0.0")]
                public __Type5F181F850177F3A0()
                {
                        InitializeComponent();
                }

                [global::System.CodeDom.Compiler.GeneratedCode("Microsoft.Maui.Controls.SourceGen", "1.0.0.0")]
#if NET5_0_OR_GREATER
#endif
                private void InitializeComponent()
                {
                        global::Microsoft.Maui.Controls.Xaml.Extensions.LoadFromXaml(this, typeof(__Type5F181F850177F3A0));
                }
        }
}

While it's likely this won't be needed, I'll try to get this resolved properly in symbolic, thus getting it fixed in CLI, symbolicator & sentry.io

Though I wonder if that might be a bug in the dotnet SDK when producing the ppdb, considering the specification says it should be "Document" when we filter "Embedded Source"...

On a side note, when do we even upload all the intermediate stuff from obj? Isn't it enough to grab bin? It has been for my testing so far.

@vaind
Copy link
Collaborator

vaind commented Jan 31, 2023

Though I wonder if that might be a bug in the dotnet SDK when producing the ppdb, considering the specification says it should be "Document" when we filter "Embedded Source"...

It's likely a bug... I tried to fetch the Document from the MethodDebugInformation table by the given MethodDef value but it's out of bounds because all indexing in PPDB is 1-based... and the value is zero...

I'll just make the change to skip these gracefully

@mattjohnsonpint
Copy link
Contributor Author

Given that we don't want a Sentry CLI upload failure to fail the build, I've changed it to log a warning instead. I've also added code to retry with sources disabled, so we still get symbols. This should be good enough until the next release of Symbolic+SentryCLI with the issue fixed.

@mattjohnsonpint
Copy link
Contributor Author

On a side note, when do we even upload all the intermediate stuff from obj? Isn't it enough to grab bin? It has been for my testing so far.

I originally chose obj because Sentry CLI found a lot more files there, especially when AOT compiling - such as native .so files. After some more testing, I think they probably aren't needed. I switched it to use the bin folder by default. But just in case, I also added a property SentryIncludeIntermediateOutputPath to opt-in to scanning the obj folder also.

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.

:shipit:

<Target Name="UploadSymbolsToSentry" AfterTargets="Build;Publish" DependsOnTargets="PrepareSentryCLI" Condition="'$(SentryUploadSymbols)' == 'true' And '$(SentryCLI)' != ''">

<Warning Condition="'$(SentryUploadSources)' == 'true' And '$(EmbedAllSources)' == 'true'"
Text="Both SentryUploadSources and EmbedAllSources are enabled. Disabling SentryUploadSources." />
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
Text="Both SentryUploadSources and EmbedAllSources are enabled. Disabling SentryUploadSources." />
Text="Both SentryUploadSources and EmbedAllSources are enabled. Ignoring SentryUploadSources." />

@mattjohnsonpint mattjohnsonpint merged commit 2c9e5d9 into main Jan 31, 2023
@mattjohnsonpint mattjohnsonpint deleted the cli-update branch January 31, 2023 22:23
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.

On MAUI sentry-cli seems to only run before AOT, missing the AOT'd symbols
4 participants