Video recording with s3 upload capability for browser nodes.

[prashanth-volvocars]

Description

Added ffmpeg directly to node container images to enable video recording controlled via 'RECORD_VIDEO' env variable.
Recorded videos are stored at location specified by 'VIDEO_LOCATION' (defaults to /tmp) env variable with format '<sesssion_id>.mp4'. Added new script that enables uploading recorded video to s3 if 'UPLOAD_TO_S3' env variable is set to 'true' to a bucket specified under 'S3_VIDEOS_BUCKET' env variable.

Motivation and Context

Currently to record video of running session, we are required to run a separate docker container. Also the video recording starts, the moment browser node and video recorder container comes up and continues until both of them are killed. So if we browser node handles multiple sessions in its lifecycle, then a single video is created combined for all the sessions. This pr will enhance the current behaviour.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[prashanth-volvocars]

@diemol @jamesmortensen. My initial thought for enabling video recording was to use "se:recordVideo": "true" as we do today in the dynamic grid but when i tried to pass this with the capabilities, I couldn't find it from the capability retrieved from the node status api. I suspect that it is because we read it only here https://github.com/SeleniumHQ/selenium/blob/f70ac4372a1a0dc57e5c710560386ff5201a890e/java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java#L410 and is not passed through if dynamic grid is not used.

[jamesmortensen]

Hi @prashanth-volvocars I'm wondering if it's possible to leveraging the existing selenium/video container images for recording video? Currently, ffmpeg is isolated from the rest of the container images for these reasons:

• to keep the images smaller for those who don't want to opt-in for video recording.
• to avoid any licensing issues with ffmpeg. (ffmpeg is GPL software).

What do you think? Any other ideas come to mind?

[prashanth-volvocars]

Hello @jamesmortensen,

The idea here is not to run separate container just for the purpose of video recording. There is also this issue of starting and stopping ffmpeg based on session which requires some additional work on the video container end.

Also i compared the image sizes before and after adding ffmpeg & aws cli. it adds around 500mb more. Further digging deep i found that aws cli take around 330mb of space and ffmpeg takes 170mb.

If size is a concern we may remove the aws cli and see if we can come up with a shell script to upload files to s3.

With regards to licencing, I am not sure what would be the solution here. I would rely on some expert advice on this matter.

[diemol]

Thank you for this PR, @prashanth-volvocars!

While I find the contents of it to be technically correct, I feel the solution is hard to maintain and cumbersome. Mainly because:

• Not everyone wants to have video recording, and adding ~170MB more is a considerable change. Pulling a browser image already takes around ~40s with a really good connection. Adding a few KB or even a couple of MB is fine, but this is a considerable change.
• If we add ffmpeg, aside of checking the licensing issues, we would be duplicating things, since we have the video image already. It is true it can only be user properly in Docker, but I believe an approach where a pod has the different images ready could be an option. In addition, if we went this way, it'd be better to change the Grid itself to start video recording instead of using bash scripts (better error control).
• I understand the need to upload the resulting video files to a location where they can be shared with the rest of the team. However, tying this to AWS is too narrow and not everyone would use it (plus it adds a bunch of MB to the image). I believe a better approach would be to have a companion service that dedicates itself to upload the session artifacts.

I am still thinking it'd be a better approach to have the standalone docker images as the ones running in pods and they could take care of starting the browser and the video container. I should probably work on that. I will leave this PR open as a way to get feedback and find ways to move forward.

[smoke]

This is quite nice! Awesome job!

[smoke]

@diemol @prashanth-volvocars To meet all of the concerns and to have all of the benefits - I believe this can be implemented using optional / configurable side-car containers with the video recorder - these will share the volumes, pid space and be able to be activated / deactivated through config.

[prashanth-volvocars]

@diemol

I have moved the session based video recording logic to the video docker image. Just wanted to get your opinion if this would work fine. I am planning to add the video image as a sidecar container as @smoke suggested.

[win5923]

Can video record be integrated into Helm chart?

[prashanth-volvocars]

Can video record be integrated into Helm chart?

Yes i am working on it as we speak. Will update the pr once done

[prashanth-volvocars]

Hi @diemol,

To address your below comments

• Not everyone wants to have video recording, and adding ~170MB more is a considerable change. Pulling a browser image already takes around ~40s with a really good connection. Adding a few KB or even a couple of MB is fine, but this is a considerable change.
• If we add ffmpeg, aside of checking the licensing issues, we would be duplicating things, since we have the video image already. It is true it can only be user properly in Docker, but I believe an approach where a pod has the different images ready could be an option. In addition, if we went this way, it'd be better to change the Grid itself to start video recording instead of using bash scripts (better error control).
• I understand the need to upload the resulting video files to a location where they can be shared with the rest of the team. However, tying this to AWS is too narrow and not everyone would use it (plus it adds a bunch of MB to the image). I believe a better approach would be to have a companion service that dedicates itself to upload the session artifacts.

I have move the logic of recording the video per session and uploading it to s3 to the video image. More over aws cli has been removed and replaced with s3cmd which is less than 1 mb in size. I hope this addresses all the above concerns. Please let me know your feedback

P.S: I am yet to update the documentation in README.md. I will do that once we agree this approach would work out.

[jamesmortensen]

Hi @diemol From what I can tell looking through this, it appears the CLI tool size issue is resolved by using an alternative tool. Also, the changes appear to be to the Video image directly, as well as Helm config files. The browser images have no changes made to them.

Now, the upload feature is still specific to AWS only, but I think that could be addressed in a future pull request if someone wanted to add additional cloud buckets. I think trying to address that now could be over-architecting.

What do you think?

[diemol]

I believe #1881 builds on this, we need to iterate on the review process.