sitespeedio / chrome-har

Create HAR files from Chrome Debugging Protocol data
MIT License
149 stars 50 forks source link

Add option to skip deleteInternalProperties() #24

Open jancurn opened 6 years ago

jancurn commented 6 years ago

It would be awesome if there was an option to keep the internal properties of the HAR entries, such as __requestId. This would allow us to make extensions to the HAR, e.g. to add the raw content of the requests. Is that something you'd consider adding? If yes, I can prepare a pull request.

soulgalore commented 6 years ago

Hi @jancurn yes that sounds cool. Do you see other use cases than requestId? I checked the HAR from WebPageTest and they expose **requestId as _request_id**, so I'm thinking we should follow the same pattern, then https://github.com/micmro/PerfCascade can use the same to show the id both for WPT and chrome-har? Ping @tobli

Best Peter

jancurn commented 6 years ago

Actually currently I only need __requestId, so the solution you're suggesting would work. If you give me green light, I'll prepare a PR that stores __requestId as _request_id.

soulgalore commented 6 years ago

Sounds good, please go ahead @jancurn :)

tobli commented 6 years ago

Hi! Late to the game, on vacation. The internal properties (prefixed with ) are implementation details that can (and have) change, so exposing them as external API is not intended. If there's precedence from WebPageTest to expose the Chrome request as _request_id however, I think that's ok to do always, and not add a preference for that. We do already add a bunch of WPT extensions, so one more field doesn't require an added preference. Note however that the the requestId for HAR entries doesn't correspond exactly to the request id from Chrome events, for redirect events the id of the previous entry is altered to add an 'r' (see https://github.com/sitespeedio/chrome-har/blob/a294335668dfca26108e57c6509a0adf698628d0/index.js#L323).