Skip to content

adds traces for dataloaders#33

Merged
PascalSenn merged 12 commits intomainfrom
pse/dataloader-spans
Feb 26, 2026
Merged

adds traces for dataloaders#33
PascalSenn merged 12 commits intomainfrom
pse/dataloader-spans

Conversation

@PascalSenn
Copy link
Contributor

No description provided.

spec/spans.yml Outdated
Comment on lines +171 to +172
The span SHOULD be a child of the span that initiated the batch loading,
which is typically a resolver execution span.
Copy link
Contributor

@EmrysMyrddin EmrysMyrddin Sep 12, 2025

Choose a reason for hiding this comment

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

You probably already discussed this during the meeting, but I'm not sure to understand this part.

What does "initiated the batch loading" ? Is it the last or the first resolver that called this DataLoader ?

And this means that other resolvers spans will not have anything to do with this span. Should we specify that the DataLoader span should be linked to every resolver spans involved in the batch ? Because a common need is to being able to track down which resolver triggered a DataLoader batch.

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

Should DataLoader even be under the graphql scope? It's generally useful, you can use it with or without GraphQL.

@PascalSenn
Copy link
Contributor Author

@benjie Fair point. They can technically be used outside of GraphQL - yet they rarely are. At least I've only seen usecases outside of the GraphQL execution by people working with a GraphQL server somewhere else in the application.

I grouped them under the graphql scope for simplicity

Base automatically changed from pse/initial-spec-definition to main February 26, 2026 06:46
@PascalSenn PascalSenn merged commit cb834ab into main Feb 26, 2026
2 checks passed
@PascalSenn PascalSenn deleted the pse/dataloader-spans branch February 26, 2026 08:17
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.

3 participants