Change FileName to be relative to project root #1739

SimonCropp · 2022-06-22T11:08:48Z

hopefully helps with #327

SimonCropp · 2022-06-22T11:10:34Z

@bruno-garcia this embeds the project and solution dir into the assembly that consumes Sentry.

so where would i wire this in to help with #327

bruno-garcia

Nice! yeah, I think we need to figure out where that goes into the payload next.
@untitaker do you know about this? (see: #327)

untitaker · 2022-06-22T20:40:45Z

What is the question?

bruno-garcia · 2022-06-22T20:44:30Z

What is the question?

We have abs_path

abs_path
The absolute path to the source file.

We currently have the rooted path there D:\bla\something\else\app\file.cs.
This PR allows us to know where app.

Do we add the 'project root path' somewhere to the payload? Or do we just truncate the D:\bla\something\else out of the abs_path and just send the relevant 'project path'? More details on: #327 (comment)

Should we have abs_path as is (FQP) and an additional, 'relative to root' path in another field? We want to allow codeowners, suspect commits, stack trace linking, etc.

bruno-garcia · 2022-06-22T20:47:29Z

I think I found the answer:

filename
The path to the source file relative to the project root directory.

https://develop.sentry.dev/sdk/event-payloads/stacktrace/#frame-attributes

I believe today we only have the file in there. Without path.

untitaker · 2022-06-28T16:22:51Z

You're correct, you want to put the relative path into filename. That's what the python sdk does at least

sorry for the slow response, i definetly replied via email but for some reason it didn't show up here

src/Sentry/AttributeReader.cs

bruno-garcia · 2022-07-11T21:38:31Z

src/Sentry/AttributeReader.cs

+    {
+        public static bool TryGetProjectDirectory(Assembly assembly, out string? projectDirectory)
+        {
+            projectDirectory = assembly.GetCustomAttributes<AssemblyMetadataAttribute>()


Is this safe, and/or does this work with AOT? Like IL2CPP and net6.0-ios for example?
Just curious if it'll at least return null or it'll throw

bruno-garcia · 2022-07-11T21:41:06Z

src/Sentry/buildTransitive/Sentry.props

+    <WriteCodeFragment AssemblyAttributes="@(SentryAttributes)"
+                       Language="$(Language)"
+                       OutputFile="$(SentryAttributesFilePath)">
+      <Output TaskParameter="OutputFile" ItemName="Compile" Condition="$(Language) != 'F#'" />


Condition="$(Language) != 'F#'" does this mean we want this for VB or a possible IronRuby or something if that's a thing?

for some reason the item name for F# is CompileBefore but for VB and c# it is Compile.

for other languages (like IronRuby) i have not tested. do you want me to include any other languages? Currently they will be a no-op

bruno-garcia · 2022-07-11T21:41:51Z

test/Sentry.Tests/AttributeReaderTests.cs

+
+public class AttributeReaderTests
+{
+    [Fact]


Cool, this should run on iOS AOT'ed so we would see how it behaves there. Assuming the tests are running there following #1703

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

mattjohnsonpint · 2022-07-13T03:34:15Z

Thats odd. It seems to be coming up blank on Mono in CI:

Error: Assert.Equal() Failure
          ↓ (pos 0)
Expected: Internals/SentryStackTraceFactoryTests.cs
Actual:   
          ↑ (pos 0)

But it works on my local machine, even on Mono. hmmm

SimonCropp · 2022-07-13T04:00:17Z

@mattjohnsonpint r u running in release mode locally?

mattjohnsonpint · 2022-07-13T15:36:33Z

Testing further, it doesn't matter if debug vs release mode. It passes on all targets when run from Rider's test runner. It fails on NET461 (Mono) when run from dotnet test on the command line.

I've seen before where I get filenames and line numbers in Rider, but that was when a debugger was attached. This is in the test runner. Whatever they are doing special, they're doing it for tests as well.

src/Sentry/Sentry.csproj

mattjohnsonpint · 2022-07-13T16:10:32Z

FYI - even if I build and test (passing) in Rider, and then test without building so it's using the exact same code (dotnet test test/Sentry.Tests --filter FileNameShouldBeRelative -f net461 --no-build), it still fails. So I don't believe it's a compiler flag, but something being done at runtime.

mattjohnsonpint · 2022-07-13T17:58:01Z

I figured it out.

Mono needs a --debug command line argument passed at runtime to emit filenames in stack traces.
Mono has a feature where the MONO_ENV_OPTIONS environment variable can be used to set any arguments that would normally be passed on the command line.
Rider is likely setting (at least) MONO_ENV_OPTIONS="--debug"

I can get the test to pass on the command line like this:

MONO_ENV_OPTIONS="--debug" dotnet test test/Sentry.Tests --filter FileNameShouldBeRelative -f net461

The question is, do we want to rely on this in our tests? It won't happen at runtime for a user application.

SimonCropp · 2022-07-13T21:01:03Z

my vote is we exclude that test for mono

untitaker · 2022-10-11T07:21:40Z

You're correct, you want to put the relative path into filename. That's what the python sdk does at least

…

On Wed, Jun 22, 2022, 22:47 Bruno Garcia ***@***.***> wrote: I think I found the answer: filename The path to the source file relative to the project root directory. https://develop.sentry.dev/sdk/event-payloads/stacktrace/#frame-attributes I believe today we only have the file in there. Without path. — Reply to this email directly, view it on GitHub <#1739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGMPRKXSXLADNOYDLL5O53VQN3WZANCNFSM5ZPZ74ZA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

add AttributeReader

7c43c6b

SimonCropp requested review from bruno-garcia and mattjohnsonpint as code owners June 22, 2022 11:08

bruno-garcia reviewed Jun 22, 2022

View reviewed changes

SimonCropp changed the title ~~add AttributeReader~~ Send project root path with events Jun 30, 2022

SimonCropp added 3 commits June 30, 2022 21:23

Merge branch 'main' into AttributeReader

b17cef2

Update CHANGELOG.md

cf02e18

.

7d1408d

SimonCropp changed the title ~~Send project root path with events~~ Change FileName to be relative to project root Jun 30, 2022

SimonCropp requested a review from bruno-garcia June 30, 2022 12:47

SimonCropp added 5 commits June 30, 2022 22:53

.

d84a50b

.

b9281cf

Merge branch 'main' into AttributeReader

d993e8f

.

d0d5219

.

69b42d6

bruno-garcia reviewed Jul 11, 2022

View reviewed changes

mattjohnsonpint and others added 2 commits July 12, 2022 18:00

Fix failing test

5cf4907

Update src/Sentry/AttributeReader.cs

52b445f

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Merge branch 'main' into AttributeReader

34783ec

mattjohnsonpint reviewed Jul 13, 2022

View reviewed changes

src/Sentry/Sentry.csproj Outdated Show resolved Hide resolved

Merge branch 'main' into AttributeReader

5939c2d

mattjohnsonpint assigned SimonCropp Jul 13, 2022

SimonCropp added 4 commits July 14, 2022 11:37

Update SentryStackTraceFactoryTests.cs

297f31d

Merge branch 'main' into AttributeReader

ff10b96

.

59ab8db

Update Sentry.props

6f62a5a

SimonCropp requested review from mattjohnsonpint and bruno-garcia July 14, 2022 10:48

nits

b09f13c

mattjohnsonpint approved these changes Jul 14, 2022

View reviewed changes

mattjohnsonpint merged commit 580c1dd into main Jul 14, 2022

mattjohnsonpint deleted the AttributeReader branch July 14, 2022 20:38

mattjohnsonpint mentioned this pull request Jul 15, 2022

Fix test failing on Windows #1782

Closed

mattjohnsonpint mentioned this pull request Sep 6, 2022

AssemblyMetadataAttribute with Sentry.ProjectDirectory and full project path is added to my executable #1900

Closed

bruno-garcia mentioned this pull request Nov 29, 2023

Removed ConfigureFromConfigurationOptions from Sentry.Azure.Functions.Worker #2920

Merged

jamescrosswell mentioned this pull request Nov 30, 2023

Generate AotHelper properties at compile time #2927

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change FileName to be relative to project root #1739

Change FileName to be relative to project root #1739

SimonCropp commented Jun 22, 2022

SimonCropp commented Jun 22, 2022

bruno-garcia left a comment

untitaker commented Jun 22, 2022

bruno-garcia commented Jun 22, 2022 •

edited

Loading

bruno-garcia commented Jun 22, 2022

untitaker commented Jun 28, 2022

bruno-garcia Jul 11, 2022

bruno-garcia Jul 11, 2022

SimonCropp Jul 14, 2022

bruno-garcia Jul 11, 2022

mattjohnsonpint commented Jul 13, 2022

SimonCropp commented Jul 13, 2022

mattjohnsonpint commented Jul 13, 2022

mattjohnsonpint commented Jul 13, 2022

mattjohnsonpint commented Jul 13, 2022

SimonCropp commented Jul 13, 2022

untitaker commented Oct 11, 2022 via email

Change FileName to be relative to project root #1739

Change FileName to be relative to project root #1739

Conversation

SimonCropp commented Jun 22, 2022

SimonCropp commented Jun 22, 2022

bruno-garcia left a comment

Choose a reason for hiding this comment

untitaker commented Jun 22, 2022

bruno-garcia commented Jun 22, 2022 • edited Loading

bruno-garcia commented Jun 22, 2022

untitaker commented Jun 28, 2022

bruno-garcia Jul 11, 2022

Choose a reason for hiding this comment

bruno-garcia Jul 11, 2022

Choose a reason for hiding this comment

SimonCropp Jul 14, 2022

Choose a reason for hiding this comment

bruno-garcia Jul 11, 2022

Choose a reason for hiding this comment

mattjohnsonpint commented Jul 13, 2022

SimonCropp commented Jul 13, 2022

mattjohnsonpint commented Jul 13, 2022

mattjohnsonpint commented Jul 13, 2022

mattjohnsonpint commented Jul 13, 2022

SimonCropp commented Jul 13, 2022

untitaker commented Oct 11, 2022 via email

bruno-garcia commented Jun 22, 2022 •

edited

Loading