Skip to content

usage reporting: allow sending traces for unexecutable operations#6194

Merged
glasser merged 2 commits into
mainfrom
glasser/trace-unexecutable
Mar 10, 2022
Merged

usage reporting: allow sending traces for unexecutable operations#6194
glasser merged 2 commits into
mainfrom
glasser/trace-unexecutable

Conversation

@glasser

@glasser glasser commented Mar 10, 2022

Copy link
Copy Markdown
Member

In #5963 we added !!metrics.captureTraces to the condition for
considering sending an operation as a trace to Studio. This makes sense
for operations which start to execute (don't send something as a trace
if we didn't record a trace for it!) but if the reason captureTraces
is unset is because we never successfully resolved an operation, it
still can be interesting to send a trace, for the sake of the Errors
page. So we allow sending traces for unexecutable operations (though it
still goes through our standard sampling algorithm).

In order to make sure the recently added Trace.fieldExecutionWeight is
set to the default value of 1 in this case, we set it when initially
creating the Trace rather than in didResolveOperation.

Fixes #6193.

@glasser glasser requested a review from trevor-scheer March 10, 2022 06:07
@codesandbox-ci

codesandbox-ci Bot commented Mar 10, 2022

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8d39d2c:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

In #5963 we added `!!metrics.captureTraces` to the condition for
considering sending an operation as a trace to Studio. This makes sense
for operations which start to execute (don't send something as a trace
if we didn't record a trace for it!) but if the reason `captureTraces`
is unset is because we never successfully resolved an operation, it
still can be interesting to send a trace, for the sake of the Errors
page.  So we allow sending traces for unexecutable operations (though it
still goes through our standard sampling algorithm).

In order to make sure the recently added Trace.fieldExecutionWeight is
set to the default value of 1 in this case, we set it when initially
creating the Trace rather than in didResolveOperation.

Fixes #6193.
@glasser glasser force-pushed the glasser/trace-unexecutable branch from 22cc5c4 to b5cf5be Compare March 10, 2022 06:16
@kevinmmartins

Copy link
Copy Markdown

I tested this PR locally and it works perfectly.
I was able to visualize in Studio the traces of these operations outside the GraphQL context.
It is very useful for us to check clients performing bad operations.

@trevor-scheer trevor-scheer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@glasser glasser merged commit c89cfde into main Mar 10, 2022
@glasser glasser deleted the glasser/trace-unexecutable branch March 10, 2022 18:04
glasser added a commit that referenced this pull request Mar 11, 2022
)

In #5963 we added `!!metrics.captureTraces` to the condition for
considering sending an operation as a trace to Studio. This makes sense
for operations which start to execute (don't send something as a trace
if we didn't record a trace for it!) but if the reason `captureTraces`
is unset is because we never successfully resolved an operation, it
still can be interesting to send a trace, for the sake of the Errors
page.  So we allow sending traces for unexecutable operations (though it
still goes through our standard sampling algorithm).

In order to make sure the recently added Trace.fieldExecutionWeight is
set to the default value of 1 in this case, we set it when initially
creating the Trace rather than in didResolveOperation.

Fixes #6193.
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Usage reporting: operations that can't execute still can make interesting traces

3 participants