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.

Sign up for GitHub

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

Jump to bottom

ref: Expose configurable stack parser #4902

Merged
merged 23 commits into from
Apr 12, 2022
Merged

ref: Expose configurable stack parser #4902

merged 23 commits into from
Apr 12, 2022

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Apr 8, 2022 •
edited
Loading

Adds a stackParser option to Options:

  /**
   * A stack parser implementation or an array of stack line parsers
   * By default, a stack parser is supplied for all supported browsers
   */
  stackParser?: StackParser | StackLineParser[];

Whenever we want access to a StackParser to use with the functions in eventbuilder we call stackParserFromOptions(options). This converts StackLineParser[] to StackParser and saves it back in the options so this conversion only occurs once.

Added Exports

@sentry/node

  • nodeStackParser

@sentry/browser

  • chromeStackParser
  • geckoStackParser
  • opera10StackParser
  • opera11StackParser
  • winjsStackParser
  • defaultStackParsers

AbhiPrasad and others added 16 commits April 7, 2022 13:32
@AbhiPrasad @Lms24
[v7] feat: Remove references to @sentry/apm (getsentry#4845)
97e6ff3 
Removes all references to `@sentry/apm`, a deprecated package we do not use anymore.
@AbhiPrasad @Lms24
[v7] feat(core): Delete API class (getsentry#4848)
57d37c2 
Removes the deprecated `API` class.
@AbhiPrasad @Lms24
[v7] feat: Delete deprecated startSpan and child methods (getsent…
424df27 
…ry#4849)

Remove deprecated methods `startSpan` and `child`.

These deprecated methods were removed in favour of `span.startChild`.
@AbhiPrasad @Lms24
feat(core): Remove whitelistUrls/blacklistUrls (getsentry#4850)
5a7f145
@AbhiPrasad @Lms24
feat(gatsby): Remove Sentry from window (getsentry#4857)
f855260
@AbhiPrasad @Lms24
feat(hub): Remove getActiveDomain (getsentry#4858)
f1e9da2 
Removes `getActiveDomain` function and corresponding type.
@AbhiPrasad @Lms24
feat(types): Remove deprecated user dsn field (getsentry#4864)
55b0570 
The `user` dsn component was renamed to `publicKey`. This PR removes that from the dsn field.
@AbhiPrasad @Lms24
feat(hub): Remove setTransaction scope method (getsentry#4865)
2fc13c6
@AbhiPrasad @Lms24
[v7] feat: Drop support for Node 6 (getsentry#4851)
20c7773
@AbhiPrasad @Lms24
[v7] feat(tracing): Rename registerRequestInstrumentation -> instrume…
244eb0e 
…ntOutgoingRequests (getsentry#4859)
@Lms24
fix(test): Increase MongoMemoryServer creation timeout (getsentry#4881)
285ca26 
Increases the creation timeout of `MongoMemoryServer` to temporarily fix Node integration test flakiness
@timfish
[v7] feat(node): Remove deprecated frameContextLines (getsentry#4884)
fb1b74f 
This PR removes the previously deprecated `frameContextLines` from `NodeOptions`.
@timfish
[v7] feat(browser): Remove top level eventbuilder exports (getsentry#…
81b10fb 
…4887)

These exports were historically used in `@sentry/electron`, but are no longer being used by the Electron SDK or the React Native SDK, so they can be removed.
@timfish
Merge branch '7.x' of https://github.com/getsentry/sentry-javascript …
a0792da 
…into 7.x
@timfish
Mostly working
79084f2
@timfish
Lint
52047b6
@timfish timfish marked this pull request as ready for review April 8, 2022 13:00
@timfish timfish mentioned this pull request Apr 8, 2022
Closed
15 tasks
@AbhiPrasad
Copy link
Member

We'll do this for node as well right?

@timfish timfish force-pushed the v7/configurable-stack-parser branch from 318e093 to 0988600 Compare April 8, 2022 15:51
@AbhiPrasad AbhiPrasad changed the title ref(browser): Expose configurable stack parser ref: Expose configurable stack parser Apr 8, 2022
@AbhiPrasad
Copy link
Member

I'm gonna hold off on this as we are prepping our first alpha: #4892

Let's get this in alongside the decision about event.stacktrace in the next alpha!

@smeubank smeubank added this to the 7.0.0 milestone Apr 11, 2022
@AbhiPrasad
Copy link
Member

@timfish let's wait until we get the backend deletion changes merged in first, it'll minimize merge conflicts and maybe make things cleaner here.

@AbhiPrasad
Copy link
Member

Deleting the backend (#4919) merged in, I think we can rebase on top of that and make changes as appropriate. Sorry for the rebase trouble!

@timfish
Copy link
Collaborator Author

timfish commented Apr 12, 2022

Sorry for the rebase trouble!

No trouble at all!

All rebased and fixed and ready to review. Do you think this approach is still good after the backend delete?

AbhiPrasad
AbhiPrasad approved these changes Apr 12, 2022
View reviewed changes
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

My only qualm with this approach is:

Whenever we want access to a StackParser to use with the functions in eventbuilder we call stackParserFromOptions(options). This converts StackLineParser[] to StackParser and saves it back in the options so this conversion only occurs once.

I think we want to move toward a world where we have two sets of options, the top level init one's and the internal client options. This also means the top level init options becomes immutable, which I think fits user mental models better.

Let's think about that after this PR gets merged in though! I'm going to start work on default integrations, so I can take a look at the options more holistically.

@AbhiPrasad AbhiPrasad merged commit f3e9384 into getsentry:7.x Apr 12, 2022
lobsterkatie pushed a commit that referenced this pull request Apr 13, 2022
@timfish @lobsterkatie
ref: Expose configurable stack parser (#4902)
8533cef 
Adds a `stackParser` option to `Options`:

```ts
  /**
   * A stack parser implementation or an array of stack line parsers
   * By default, a stack parser is supplied for all supported browsers
   */
  stackParser?: StackParser | StackLineParser[];
```

Whenever we want access to a `StackParser` to use with the functions in `eventbuilder` we call `stackParserFromOptions(options)`. This converts `StackLineParser[]` to `StackParser` and saves it back in the options so this conversion only occurs once.

### Added Exports
`@sentry/node` 
- `nodeStackParser`

`@sentry/browser` 
- `chromeStackParser`
- `geckoStackParser`
- `opera10StackParser`
- `opera11StackParser`
- `winjsStackParser`
- `defaultStackParsers`
Lms24 pushed a commit that referenced this pull request Apr 26, 2022
@timfish @Lms24
ref: Expose configurable stack parser (#4902)
a8e14b5 
Adds a `stackParser` option to `Options`:

```ts
  /**
   * A stack parser implementation or an array of stack line parsers
   * By default, a stack parser is supplied for all supported browsers
   */
  stackParser?: StackParser | StackLineParser[];
```

Whenever we want access to a `StackParser` to use with the functions in `eventbuilder` we call `stackParserFromOptions(options)`. This converts `StackLineParser[]` to `StackParser` and saves it back in the options so this conversion only occurs once.

### Added Exports
`@sentry/node` 
- `nodeStackParser`

`@sentry/browser` 
- `chromeStackParser`
- `geckoStackParser`
- `opera10StackParser`
- `opera11StackParser`
- `winjsStackParser`
- `defaultStackParsers`
lobsterkatie pushed a commit that referenced this pull request Apr 26, 2022
@timfish @lobsterkatie
feat: Expose configurable stack parser (#4902)
c2a57a7 
Adds a `stackParser` option to `Options`:

```ts
  /**
   * A stack parser implementation or an array of stack line parsers
   * By default, a stack parser is supplied for all supported browsers
   */
  stackParser?: StackParser | StackLineParser[];
```

Whenever we want access to a `StackParser` to use with the functions in `eventbuilder` we call `stackParserFromOptions(options)`. This converts `StackLineParser[]` to `StackParser` and saves it back in the options so this conversion only occurs once.

### Added Exports
`@sentry/node`
- `nodeStackParser`

`@sentry/browser`
- `chromeStackParser`
- `geckoStackParser`
- `opera10StackParser`
- `opera11StackParser`
- `winjsStackParser`
- `defaultStackParsers`
lobsterkatie pushed a commit that referenced this pull request Apr 26, 2022
@timfish @lobsterkatie
feat: Expose configurable stack parser (#4902)
6f09dd7 
Adds a `stackParser` option to `Options`:

```ts
  /**
   * A stack parser implementation or an array of stack line parsers
   * By default, a stack parser is supplied for all supported browsers
   */
  stackParser?: StackParser | StackLineParser[];
```

Whenever we want access to a `StackParser` to use with the functions in `eventbuilder` we call `stackParserFromOptions(options)`. This converts `StackLineParser[]` to `StackParser` and saves it back in the options so this conversion only occurs once.

### Added Exports
`@sentry/node`
- `nodeStackParser`

`@sentry/browser`
- `chromeStackParser`
- `geckoStackParser`
- `opera10StackParser`
- `opera11StackParser`
- `winjsStackParser`
- `defaultStackParsers`
@timfish timfish deleted the v7/configurable-stack-parser branch May 19, 2022 13:14
AbhiPrasad pushed a commit that referenced this pull request May 30, 2022
@timfish @AbhiPrasad
feat: Expose configurable stack parser (#4902)
4077204 
Adds a `stackParser` option to `Options`:

```ts
  /**
   * A stack parser implementation or an array of stack line parsers
   * By default, a stack parser is supplied for all supported browsers
   */
  stackParser?: StackParser | StackLineParser[];
```

Whenever we want access to a `StackParser` to use with the functions in `eventbuilder` we call `stackParserFromOptions(options)`. This converts `StackLineParser[]` to `StackParser` and saves it back in the options so this conversion only occurs once.

### Added Exports
`@sentry/node`
- `nodeStackParser`

`@sentry/browser`
- `chromeStackParser`
- `geckoStackParser`
- `opera10StackParser`
- `opera11StackParser`
- `winjsStackParser`
- `defaultStackParsers`
@Lms24 Lms24 mentioned this pull request Jul 29, 2022
Merged
Lms24 added a commit that referenced this pull request Aug 1, 2022
@Lms24
fix(node): Ensure that self._handler exists before calling it in Link…
101a023 
…edErrors (#5497)

Bring back a definedness check for `self._handler` in the global event processor of the LinkedErrors integration for Node. 
The check was removed in #4902 but apparently it is still necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@timfish @AbhiPrasad @smeubank @Lms24