Skip to content

Conversation

david-luna
Copy link
Member

Checklist

Copy link

github-actions bot commented Sep 8, 2025

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@david-luna david-luna changed the title chore: update to v5 chore: update @apollo/server to v5 Sep 8, 2025
var name =
queries.length > 0 ? queries.join(', ') : 'Unknown GraphQL query';
if (trans.req) var path = getPathFromRequest(trans.req, true);
if (trans.req) var path = getPathFromRequest(trans.req, true, true);
Copy link
Member Author

Choose a reason for hiding this comment

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

note to reviewer: The 3rd param is to use the path as transaction name and makes the test pass but I?m not 100% sure about the implications of changing this.

Copy link
Member

Choose a reason for hiding this comment

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

I don't either. The tests are only testing cases where the path is '/', so kinda hard to know if there are real implications.

usePathAsTransactionName is in general dangerous because it can lead to high cardinality transaction names. But I'm not sure if that is a real concern for GraphQL endpoints where I think the GraphQL calls are all hanging off of a single HTTP path.

@david-luna david-luna marked this pull request as ready for review September 25, 2025 17:07
@david-luna david-luna requested a review from a team as a code owner September 25, 2025 17:07
Copy link
Member Author

@david-luna david-luna left a comment

Choose a reason for hiding this comment

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

/test tav @apollo/server

Copy link

🔍 Preview links for changed docs

Copy link
Member Author

@david-luna david-luna left a comment

Choose a reason for hiding this comment

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

/test tav @apollo/server

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.

2 participants