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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BUG: Profiling doesn't work as expected #907

Closed
threepointone opened this issue May 5, 2022 · 11 comments 路 Fixed by #908 or #909
Closed

馃悰 BUG: Profiling doesn't work as expected #907

threepointone opened this issue May 5, 2022 · 11 comments 路 Fixed by #908 or #909
Assignees
Labels
bug Something that isn't working internal Requires support from the Cloudflare Platform

Comments

@threepointone
Copy link
Contributor

What version of Wrangler are you using?

0.0.30

What operating system are you using?

Mac

Describe the Bug

Profiling as described in https://developers.cloudflare.com/workers/learning/profiling-workers/ doesn't work with wrangler v2. The button to start profiling is disabled in devtools. (Whether opened via chrome://inspect, or the standalone tools on hitting D).

@threepointone threepointone added this to the 2.0 milestone May 5, 2022
@threepointone
Copy link
Contributor Author

More info:

In chrome://inspect, the button gets enabled after visiting the worker at least once. Dug in a little, and it looks like the prewarm logic isn't working . This code - https://github.com/cloudflare/wrangler2/blob/f2b6cd94b354d44a88298388ccd9ccec710ed093/packages/wrangler/src/create-worker-preview.ts#L169-L171 is always erroring.

In the standalone/hotkey tools, the button gets enabled, but doesn't look to ever stop after clicking Stop.

threepointone added a commit that referenced this issue May 5, 2022
When calling `wrangler dev`, we make a request to a special URL that "prewarms" the isolate running our Worker so that we can attach devtools etc to it before actually making a request. We'd implemented it wrongly, and because we'd silenced its errors, we weren't catching it. This patch fixes the logic (based on wrangler 1.x's implementation) and enables logging errors when the prewarm request fails.

As a result, profiling starts working again as expected. Fixes #907
threepointone added a commit that referenced this issue May 5, 2022
When calling `wrangler dev`, we make a request to a special URL that "prewarms" the isolate running our Worker so that we can attach devtools etc to it before actually making a request. We'd implemented it wrongly, and because we'd silenced its errors, we weren't catching it. This patch fixes the logic (based on wrangler 1.x's implementation) and enables logging errors when the prewarm request fails.

As a result, profiling starts working again as expected. Fixes #907
@threepointone
Copy link
Contributor Author

Reopening this since it looks like the Profiling itself isn't working, it doesn't. Via Discord:

Also the cpu profiler doesn't seem to work in devtools. I click record/start, then make a request to my worker (which completes normally), then I stop the profiling. Then it shows that my worker been idle for the whole time. I've tried it a few times now and I'm 100% sure that I'm making a request to my worker (I see console logs in my terminal and devtools) while it's recording. I only see idle in the profiler

image

@threepointone threepointone reopened this May 13, 2022
@threepointone threepointone modified the milestones: 2.0, 2.1 May 13, 2022
@petebacondarwin petebacondarwin modified the milestones: Selected for development, Backlog May 15, 2022
@petebacondarwin petebacondarwin added bug Something that isn't working awaiting reporter response Needs clarification or followup from OP and removed type: bug labels May 16, 2022
@petebacondarwin
Copy link
Contributor

petebacondarwin commented May 16, 2022

@threepointone to double check whether this even works in the 潭d潭a潭s潭h潭b潭o潭a潭r潭d潭 native devtools.

@threepointone
Copy link
Contributor Author

I can confirm that profiling is broken at an api/edge level. I verified that profiling doesn't "work" with wrangler 1 as well, so there's nothing to fix here, and we should follow up internally. I'll leave this assigned to me, but marking as blocked for now.

@threepointone threepointone added blocked Blocked on other work internal Requires support from the Cloudflare Platform and removed awaiting reporter response Needs clarification or followup from OP labels May 17, 2022
@petebacondarwin
Copy link
Contributor

Related (sort of) to #3

@Warfields
Copy link
Contributor

@threepointone I've taken a look at this. The spec indicates that the runtime will only send CPU Stats in local fiddle, apparently there were concerns about timing data and Spector. I'll be bridging a discussion of if it is ok to open this up on the edge.

@Warfields
Copy link
Contributor

@petebacondarwin @threepointone I'm asking the Runtime team on Wednesday

@Warfields
Copy link
Contributor

Update, found the bug in the runtime. V8 changed how it returns somethings from the profiler.

@threepointone
Copy link
Contributor Author

assigning to @Warfields

@threepointone threepointone removed their assignment Jul 1, 2022
@Warfields
Copy link
Contributor

@threepointone A fix has been merged to the runtime and is awaiting release. As soon as the next release this bug can be closed

CC: @JacobMGEvans As Sunil is out

@Warfields Warfields removed the blocked Blocked on other work label Aug 9, 2022
@ObsidianMinor
Copy link
Contributor

The fix has been released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working internal Requires support from the Cloudflare Platform
Projects
Archived in project
4 participants