Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/instrumentation/modules/graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ module.exports = function (graphql, agent, { version, enabled }) {
if (trans._graphqlRoute) {
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.

var defaultName = name;
defaultName = path ? defaultName + ' (' + path + ')' : defaultName;
defaultName = operationName
Expand Down
Loading
Loading