fix: set operation_name tracing attribute to empty on unnamed, single-op requests
#525
Conversation
…le-op requests When GraphQL operation names are not nececessary to execute an operation (i.e., when there is only a single operation in a GraphQL document) and the GraphQL operation is _not_ named (i.e., it is anonymous), the `operation_name` attribute on the trace spans that are associated with the request will no longer contain a single hyphen character (`-`) but will instead be an empty string. This matches the way that these operations are represented during the GraphQL operation's life-cycle as well. Fixes #481
abernix
force-pushed
the
abernix/issue-481
branch
from
c67e68a
to
adb7bdd
Compare
| @@ -345,7 +345,7 @@ where | |||
| name = "graphql_request", | |||
| fields( | |||
| query = %request.query.clone().unwrap_or_default(), | |||
| operation_name = %request.operation_name.clone().unwrap_or_else(|| "-".to_string()), | |||
| operation_name = %request.operation_name.clone().unwrap_or_else(|| "".to_string()), | |||
There was a problem hiding this comment.
That's going to break the studio usage though. If you look at stats_report_key() in apollo_open you'll note that we assume the default name for an operation there is "-" which is what we want for stats reporting.
To keep things working, you'll need to modify stats_report_key() to check if op_name is "" and, if it is, then update that logic to substitute "-" as the op_name. The comment there will need to be updated as well.
There was a problem hiding this comment.
Ah, that makes sense. Thanks! I was under the impression that this was added within the APQ issue (since that's where the review comment was) and had assumed we were doing the default for Studio reporting elsewhere.
There was a problem hiding this comment.
There should have been a comment above that instrument block to explain why we did that...
As noted in [comment][1], the previous [commit][2] I made wasn't sufficient to avoid exposing the (meant to be internal) implementation detail of the `-` (hyphen) character being used as a representation for "anonymous operation" since all my previous comment did was remove it from the tracing span attribute where it is later extracted from by Spaceport. This commit takes things a step further, and updates the `stats_report_key` method to take a required first parameter of a string. That string _may_ be empty, but empty merely matches the expectation of unnamed operation. Keep in mind that operations only MUST be named when there are multiple operations in the document. In the case of multiple operations, the out-of-band `operationName` parameter must be passed to the request (either via POST-body or GET query parameters). [1]: #525 (comment) [2]: adb7bdd
When GraphQL operation names are not nececessary to execute an operation (i.e., when there is only a single operation in a GraphQL document) and the GraphQL operation is not named (i.e., it is anonymous), the
operation_nameattribute on the trace spans that are associated with the request will no longer contain a single hyphen character (-) but will instead be an empty string. This matches the way that these operations are represented during the GraphQL operation's life-cycle as well.Fixes #481