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

Regression in DevTool Integration with FLEDGE API #499

Open
zhengweiwithoutthei opened this issue Mar 28, 2023 · 15 comments
Open

Regression in DevTool Integration with FLEDGE API #499

zhengweiwithoutthei opened this issue Mar 28, 2023 · 15 comments
Labels
Non-breaking Feature Request Feature request for functionality unlikely to break backwards compatibility

Comments

@zhengweiwithoutthei
Copy link

zhengweiwithoutthei commented Mar 28, 2023

I would like to bring to your attention some concerns that we have regarding the integration of DevTools with the FLEDGE API. While we have previously filed issues #387, #389 on Github requesting improvements in this area, we have noticed some regressions that are making it difficult for us to debug and discover issues. Specifically,

  1. We have observed that FLEDGE-related network requests such as requests to fetch seller and buyer scripts, trusted server calls, and Fenced Frame reporting pings are disappearing from both the network and performance tabs in DevTools. This is a major setback as we heavily rely on DevTools to debug and measure request latency and payload. Additionally, we also have automated profiling tools that rely on these features to work. We request that these network calls be restored to the network/performance tab. chrome://net-log is feasible but adds significant friction to the debugging flow.

  2. We also ask that those network requests be exposed in the ResourceTiming API .This would allow us to measure and attribute latency to individual network callouts at scale, greatly improving our ability to identify bottlenecks and enhance performance.

  3. Furthermore, we have noticed that console.log from seller and buyer scripts is missing from the selenium webdriver logs. This is a crucial feature that we previously had access to when debugging regression tests , and we would greatly appreciate it if it could be restored.

  4. Cmd+shift+r hard refresh should also force cache renewal for decision/bidding logic script.

@JensenPaul
Copy link
Collaborator

@morlovich might want to take a look

@shivanigithub
Copy link
Contributor

@gtanzer for reportEvent beacons dev tools support

@morlovich
Copy link
Collaborator

So I am going to address (1) only; other things likely need totally separate approach.

Thanks for the report.. First I want to clarify one thing: "seller and buyer scripts, trusted server calls" have never appeared in the network tab[1], but should indeed show up in the network portion of the performance tab (they use different data sources). I have tried to reproduce it, but I am actually seeing something far worse: the worklet process crashes when trying to collect performance information. I am wondering if you could perhaps confirm if you're actually seeing that as well?

[1] It's a TODO entry to implement that, so thanks for giving a signal that people actually do care.

@dmdabbs
Copy link
Contributor

dmdabbs commented Apr 10, 2023

Thanks @morlovich. When the network tab does include FLEDGE-related calls, consider a convenience filter, say "Auction" or similar after Wasm Manifest Other. Publisher adops teams would probably appreciate a convenient means to focus on these.

@morlovich
Copy link
Collaborator

OK, I can confirm the separate bug with just the network stuff, which is obviously less important than the crashing; an unverified guess is that it's because of
https://chromium-review.googlesource.com/c/chromium/src/+/4129799
combined with logic in
third_party/devtools-frontend/src/front_end/models/timeline_model/TimelineModel.ts
... which looks for the old thread name.

@zhengweiwithoutthei
Copy link
Author

@morlovich Thanks for looking into this issue. I did not observe a crash while collecting the performance trace.
image

Yes, it would be even better if FLEDGE-related calls can also show up in the network tab.

copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this issue Apr 12, 2023
This got changed in
https://chromium-review.googlesource.com/c/chromium/src/+/4129799,
so some of these threads weren't found, resulting in WICG/turtledove#499

Bug: none
Change-Id: I59a235ed3ddeb5c6e2afbefaf8449f3ef4833f87
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4414780
Reviewed-by: Victor Porof <victorporof@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
@morlovich
Copy link
Collaborator

Both the crash and lack of network stuff in performance tab should be fixed as of 114.0.5712.0
(The crash I think shouldn't affect stable releases, but I don't understand all of the build configuration stuff involved).

@dmdabbs
Copy link
Contributor

dmdabbs commented Apr 14, 2023

Confirmed fixed in Version 114.0.5714.0 (Official Build) canary (64-bit) and following the FLEDGE Demo

fledge-performance-screenshot

  1. Should one attribute the gap between DOMContentLoaded (when runAdAuction() is called) and when auction script downloads begin to auction environment/worklet setup?

  2. Why is the dsp's bidding-logic.js downloaded a second time?

copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this issue Apr 24, 2023
This got changed in
https://chromium-review.googlesource.com/c/chromium/src/+/4129799,
so some of these threads weren't found, resulting in WICG/turtledove#499

Bug: 1432954
Change-Id: I59a235ed3ddeb5c6e2afbefaf8449f3ef4833f87
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4414780
Reviewed-by: Victor Porof <victorporof@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
(cherry picked from commit 4a76ee9)
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4436689
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
@zhengweiwithoutthei
Copy link
Author

Thanks. Is it possible to make (2) happen? It will be really useful to help us break down the network latency.

@zhengweiwithoutthei
Copy link
Author

friendly ping on (2) or any suggestion on how to measure the dsp request latency from clientside pov?

@JacobGo
Copy link
Contributor

JacobGo commented Jun 1, 2023

Bumping (4) for supporting the existing DevTools controls to disable caching for FLEDGE resources. We've observed frequent confusion here when iterating on local changes to seller and buyer JS; it would be great to respect local browser overrides around caching behavior. Thanks!

@morlovich
Copy link
Collaborator

re: (2): private aggregation has a metric for that, and that should work starting on M115 (though I am not sure it's the same definition you want). Exposing it in other ways will be tricky.

re: (4): thanks for feedback. I think I may be able to put something together for the hard-reload thing at least fairly quickly, though I may need to pull in an expert (I have no idea on how to test it properly, oops). Devtools checkbox may be somewhat more involved

aarongable pushed a commit to chromium/chromium that referenced this issue Jun 7, 2023
If the user has reloaded a page asking for all subresources
to be reloaded from network, we should extend this to FLEDGE
resources, too.

This required memorizing the reload mode in RenderFrameHostImpl,
since it previously didn't survive past NavigationRequest.

See report WICG/turtledove#499, issue #4

Bug: 1452127

Change-Id: If017a7b1e59155058cbe59e84341003e75604f30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4590272
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Caleb Raitto <caraitto@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1154577}
@morlovich
Copy link
Collaborator

Shift-reload should work as of 116.0.5820.0

@gtanzer
Copy link
Contributor

gtanzer commented Jun 9, 2023

Fenced frame reporting beacons should now be visible in the dev tools network tab, as of M115. (Might take a little time for the merge to take effect in Beta, but it's definitely working in Canary already.)

@zhengweiwithoutthei
Copy link
Author

Thanks for the updates! @morlovich @gtanzer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non-breaking Feature Request Feature request for functionality unlikely to break backwards compatibility
Projects
None yet
Development

No branches or pull requests

7 participants