-
Notifications
You must be signed in to change notification settings - Fork 291
Description
Area(s)
area:graphql
What's the problem?
The graphql.document is user-inputted and unspecified in length (potentially megabytes+ long), and due to the use of string and numeric literals (failure to use variables) it has more high-cardinality than the existing graphql.operation.name (which is already warned against) but also the risk of containing sensitive information which is difficult to fully sanitize.
For this reason, graphql.document is a liability to have listed as Recommended without serious infrastructure considerations and needs, often that are beyond the requirements of most users, making Opt-In a better option with a lower risk profile. In our customer adoption of OpenTelemetry at Apollo (in the Apollo Router), we've found our GraphQL customers unwittingly following this currently published configuration/instruction from these conventions while not understanding the full implications. We can mitigate against some of this in our own documentation and implementation, but a more opinionated "upstream" stance is worth considering. (For example, in our own implementation we can defend against operation name length to reasonable defaults (e.g., max operation name of 255 characters), but graphql.document is harder to be opinionated about without compromising the executability of the operation).
Describe the solution you'd like
In terms of debugability, in most cases, the lesser-liability of graphql.operation.name (the actual operation name) is sufficient to track usage since in many GraphQL deployments there is generally correlation between the two (in client code-bases) which can be fetched out of band. This also spares the transmission of a large payload of the operation body to metric providers.
That said, the operation name (graphql.operation.name) isn't entirely without its risk, but it's more likely to be dozens of bytes of a limited character set rather than dozens, hundreds or potentially thousands of kilobytes. In that regard, it's reasonable that graphq.operation.name be left as Recommended, though the argument could easily be made that it should also be Opt-In.
At this time, I recommend:
graphql.documentbecomeOpt-Inon account of it's more serious liabilities (including sensitive information redaction) which are not straightforward to defend against or mitigate in most deploymentsgraphql.operation.nameremainRecommended, though should still remain disclaimed in regards to its high-cardinality/user-input concerns (which are already noted).
I am happy to open a PR.
Tip
React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status