Skip to content

Node stack filenames should be relative to app root #9072

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
Tracked by #14149
timfish opened this issue Sep 20, 2023 · 0 comments
Open
Tracked by #14149

Node stack filenames should be relative to app root #9072

timfish opened this issue Sep 20, 2023 · 0 comments

Comments

@timfish
Copy link
Collaborator

timfish commented Sep 20, 2023

Problem Statement

Currently, the Node SDK captures absolute paths for stack trace filenames.

This has a number of disadvantages:

  • Change of hosting config or provider might change the paths
  • Paths differ by OS/username

These differences can impact error grouping.

Solution Brainstorm

For the Electron SDK the OS/username in paths was a major issue so we already cater for this by normalising paths to the application root.

We should do something similar for the Node SDK but this will be a breaking change since it would affect customer error grouping.

@AbhiPrasad AbhiPrasad added this to the 8.0.0 milestone Sep 20, 2023
AbhiPrasad pushed a commit that referenced this issue Jan 3, 2024
This PR makes a few fixes and additions to the ANR feature.
- Stops passing the `sdk` property with the event since
`enhanceEventWithSdkInfo` results in duplicate integrations/packages
being sent!
- Allows the passings of `staticTags` to attach to ANR events 
- Required by Electron SDK to tag the source
`process/origin/environment`
  - Could also be useful for other users
- Optionally enable normalising of stack frame paths to the app root
  - Required for Electron
  - Soon required for Node with #9072

The path normalisation code (and tests) are from the Electron SDK and is
well tested on all platforms. However, it will only be called when
`appRootPath` is supplied. If/when we add path normalisation to Node, it
will have a default which can be overridden.

The Electron SDK will then wrap the Node Anr integration something like
this:
```ts
class Anr extends NodeAnr {
  public constructor(options: Partial<Options> = {}) {
    super({
      ...options,
      staticTags: {
        'event.environment': 'javascript',
        'event.origin': 'electron',
        'event.process': 'browser',
        ...options.tags,        
      },
      appRootPath: app.getAppPath(),
    });
  }
}
```
anonrig pushed a commit that referenced this issue Jan 3, 2024
This PR makes a few fixes and additions to the ANR feature.
- Stops passing the `sdk` property with the event since
`enhanceEventWithSdkInfo` results in duplicate integrations/packages
being sent!
- Allows the passings of `staticTags` to attach to ANR events 
- Required by Electron SDK to tag the source
`process/origin/environment`
  - Could also be useful for other users
- Optionally enable normalising of stack frame paths to the app root
  - Required for Electron
  - Soon required for Node with #9072

The path normalisation code (and tests) are from the Electron SDK and is
well tested on all platforms. However, it will only be called when
`appRootPath` is supplied. If/when we add path normalisation to Node, it
will have a default which can be overridden.

The Electron SDK will then wrap the Node Anr integration something like
this:
```ts
class Anr extends NodeAnr {
  public constructor(options: Partial<Options> = {}) {
    super({
      ...options,
      staticTags: {
        'event.environment': 'javascript',
        'event.origin': 'electron',
        'event.process': 'browser',
        ...options.tags,        
      },
      appRootPath: app.getAppPath(),
    });
  }
}
```
@AbhiPrasad AbhiPrasad modified the milestones: 8.0.0, 9.0.0 Mar 14, 2024
@lforst lforst removed this from the 9.0.0 milestone Nov 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

5 participants