feat: Make it possible to get the active span in the GraphQL resolver #2323
Conversation
🦋 Changeset detectedLatest commit: 68c2246 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/plugins/sentry/src/index.ts
Outdated
| @@ -131,56 +132,93 @@ export const useSentry = <PluginContext extends Record<string, any> = {}>( | |||
| ...addedTags, | |||
| }; | |||
|
|
|||
| let rootSpan: Span | undefined; | |||
| const createExecuteFn = async ( | |||
There was a problem hiding this comment.
The name of this function is a bit misleading.
What do you think of executeWithSpan ?
| const createExecuteFn = async ( | |
| const executeWithSpan = async ( |
.changeset/clever-rice-perform.md
Outdated
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@envelop/sentry': minor | |||
There was a problem hiding this comment.
I didn't noticed, but this PR contains a breaking change, so the version bump should be major
| '@envelop/sentry': minor | |
| '@envelop/sentry': major |
The breaking change is that now, this plugin wraps the execution phase. So the plugin is now order dependent in the plugin list. Which means it can break existing configuration.
Can you add an advice to put the plugin at the end of the plugin list ? This way, it ensures that the execute function doesn't get overwritten by another plugin.
There was a problem hiding this comment.
I have changed it to a major update and added a note that it is a breaking change.
9fb31e0
|
Some modifications have been made to this plugin on the main branch, so I merged the changes into your branch. Can you confirm it's still doing what you needed ? |
|
@EmrysMyrddin |
|
Great ! Let's merge this then :-) In fact, it's pretty much exactly what remained of your code once all changes related to deleted code on main have been removed :-P So yeah, it's basically the whole plugin that was a bit messy, because of the way legacy Sentry API used to work :-) |
|
@Karibash Can you rebase this PR ? I don't have the rights to force push your branch :/ |
Karibash
force-pushed
the
feature/active-span
branch
from
def90b5 to
bfae518
Compare
Karibash
force-pushed
the
feature/active-span
branch
from
bfae518 to
6ea2fe2
Compare
|
@EmrysMyrddin I have rebased and consolidated the commits into one. Please check it! |
|
I was hoping that it would fix the CI... I will check why it's not working |
Description
Please include a summary of the change and which issue is fixed. Please also include relevant
motivation and context. List any dependencies that are required for this change.
Fixes #2322
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Test Environment:
Checklist:
CONTRIBUTING doc and the
style guidelines of this project