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

Providing the recording as a URL query param instead of a file #3

Open
Zac-HD opened this issue Feb 16, 2021 · 7 comments
Open

Providing the recording as a URL query param instead of a file #3

Zac-HD opened this issue Feb 16, 2021 · 7 comments

Comments

@Zac-HD
Copy link
Contributor

Zac-HD commented Feb 16, 2021

I'd love to be able to use pytrace to debug failures from CI... but getting files off CI and then onto my local machine before I can view them is a pain.

Would it be possible to pass this data as a URL query param instead? If so, dumping that URL in the logs would make time-travelling debugging as easy as clicking a link 🤩

@gleb-sevruk
Copy link
Owner

Are we talking about WebUI ability to open remote recording?

I guess it is already implemented, try
https://app.pytrace.com/?open=https%3A%2F%2Fpytrace-demo.s3.eu-central-1.amazonaws.com%2Fsession.chunked

The few culprits are:

Code for reference: https://github.com/gleb-sevruk/pycrunch-tracing-webui/blob/master/src/store/index.js#L184

@Zac-HD
Copy link
Contributor Author

Zac-HD commented Feb 16, 2021

I was thinking "just click https://app.pytrace.com/?data=base64_encoded_payload_here "; i.e. putting the whole recording (compressed and encoded) in the URL. No data storage at all!

A similar idea would be to load the recording from a data-URL, like data:application/octet-stream;base64,SGVsbG8sIFdvcmxkIQ==. Same goal - make it easy to use pytrace recordings from systems where you only get text logs - though a somewhat more complicated user experience.

@gleb-sevruk
Copy link
Owner

gleb-sevruk commented Feb 16, 2021

Ok, I ran a few checks. Previously I was thinking that url length limited to 32k bytes; but now Chrome states it can support up to 2MB url string (https://chromium.googlesource.com/chromium/src/+/master/docs/security/url_display_guidelines/url_display_guidelines.md#URL-Length)

I tried compressing relatively small recording of 36MB, and it won't fit into limit:
image

As I agree this feature may be useful, it will not work under all circumstances. (what should recorder do, if predicted compressed size will exceed 2MB?)

Maybe it worth looking into alternatives, such as S3-compatible storage (Ceph, Minio, AWS) or just exposing artifacts from CI

@Zac-HD
Copy link
Contributor Author

Zac-HD commented Feb 16, 2021

My thoughts on this are basically "can we make the data smaller", then "can we compress it better" (e.g. sorting it, canonicalising, using zstandard instead of zip, etc), and if that's too hard just tell me that the feature I want isn't practical!

(in any case, my sincere thanks for an awesome open-source tool and your quick responses here 🥰)

@Zac-HD
Copy link
Contributor Author

Zac-HD commented Apr 30, 2021

The obvious alternative is local recordings, so I experimented with that for a bit. My goal is to have a link which I can print to the terminal from pytest, and the user can click to load directly into the debugger for that test. (asking the user to load it is pretty nasty, since there might be hundreds of recordings)

  1. I tried passing local filepaths as the ?open= param, but promptly ran into an XSS issue: xhr.js:178 Not allowed to load local resource: file:///C:/Users/Zac/Documents/GitHub/hypothesis/pycrunch-recordings/run/session.chunked.pycrunch-trace. OK, fair enough, allowing JS to open local files would be a horrendous security problem for Chrome.
  2. Next idea, spin up python -m http.server to serve the files over http. No dice: ... No 'Access-Control-Allow-Origin' header is present on the requested resource. Also fair, and this one I can work around by fixing the local server.

@gleb-sevruk
Copy link
Owner

gleb-sevruk commented Apr 30, 2021

python -m http.server I was thinking this too in order for integrate PyCrunch test run with time travel debugger 😀

But the obvious problem is disk space

@Zac-HD
Copy link
Contributor Author

Zac-HD commented May 1, 2021

Well, the trace has to be stored somewhere, so uploading it just moves the problem around (and is impractical for people on slow internet connections). Some combination of compressing traces and aggressively deleting old trace files would probably be sufficient.

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

No branches or pull requests

2 participants