Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve terminating callbacks visibility #707

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

stayallive
Copy link
Collaborator

@stayallive stayallive commented Jun 4, 2023

Code using something like dispatch(function () { ... })->afterResponse() did not show up in the performance traces. Even though the user wouldn't notice the impact on the response time I feel like we should still provide insight in it.

In another PR I'm working on I'll try to give better context on where the request actually ended (response sent) and where the terminating timeframe begins etc., coming soon ™️

Before:

image

After (notice the "new" queue.process span):

image

@cleptric cleptric merged commit de3d2e0 into master Jun 12, 2023
@cleptric cleptric deleted the improve-terminating-visibility branch June 12, 2023 17:56
@diogogomeswww
Copy link

On our case we don't want to include the "afterResponse" in the performance trace.
Because of that we are force to downgrade this package version.

Can we make this configurable?

I'm happy to do a PR.

@stayallive
Copy link
Collaborator Author

@diogogomeswww can you eloborate why not? What is the downside for you?

@diogogomeswww
Copy link

We track the performance of some endpoints, and we want to track the performance through the users perspective.
Hence we are sending some "work" to be dispatched after the response.

Does that make sense?

@cleptric
Copy link
Member

cleptric commented Jul 11, 2023

Having a config to opt-out of this seems reasonable to me. Out of curiosity, do you exclusively use blade templates or other SSR technologies? I would assume to measure performance from a user-perspective rather from the FE side.

@diogogomeswww
Copy link

We have API Endpoints that need to be fast.
And all work on those API Endpoints that is not needed to return the response to the user, is done using the dispatchAfterResponse.

Please give me a thumbs up and I will do the PR to add a config variable to include or exclude the dispatchAfterResponse in the performance trace.

@cleptric
Copy link
Member

We'll take care of this, no worries 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants