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

Created property keepEvents at bbb-web to make redis record the event… #6385

Merged
merged 4 commits into from
Jan 25, 2019

Conversation

pedrobmarin
Copy link
Collaborator

…s for all meetings

There are some scenarios where we would like to keep the events from all meetings. This is very useful to generate better usage reports for users.

The way we integrated this here is with a new property called keepEvents in bbb-web that can be enabled/disabled.

When enabled, bbb-web and akka-apps will use Redis' storage workflow to record events from all meetings (both recordable and not recordable). bbb-web will tag the meetings as ended and rap-archive will take an extra step digesting those events and saving in a separated folder /var/bigbluebutton/events. If the meeting was a not recordable meeting nothing else will happen in the recording process.

When keepEvents is disabled (it's default value) nothing changes from regular system behavior.

redis = BigBlueButton::RedisWrapper.new(redis_host, redis_port)
events_archiver = BigBlueButton::RedisEventsArchiver.new redis
events_xml = "#{events_dir}/#{meeting_id}/events.xml"
events = events_archiver.store_events(meeting_id, events_xml, break_timestamp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling the store_events function multiple times with the same break timestamp is not safe, because it deletes the events for the segment after it writes the events.xml file.

You must ensure that store_events is only called once per break point.

In the case of a recorded meeting, I suggest running the normal archive script then copy events.xml to the events directory. Only call the events/events.rb script on non-recorded meetings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend leaving that manual event removal code. That is supposed to be done after the final recording segment is saved - it removes some extra things in addition to events like meeting metadata and breakout room information.

var props = msg.body.props.copy()
if (props.recordProp.keepEvents) {
val recordProp = msg.body.props.recordProp.copy(record = true)
props = msg.body.props.copy(recordProp = recordProp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sets the meeting as being recorded which would show the record interface on the client. Doesn't it?

I suggest to not override the record boolean but instead pass the keepEvent flag into the OutMsgRouter and let that decide to record the message or not.

https://github.com/bigbluebutton/bigbluebutton/blob/master/akka-bbb-apps/src/main/scala/org/bigbluebutton/core/running/OutMsgRouter.scala#L9

https://github.com/bigbluebutton/bigbluebutton/blob/master/akka-bbb-apps/src/main/scala/org/bigbluebutton/core/running/RunningMeeting.scala#L47

This way the client doesn't have to know if the server is recording the events or not as it is not the client's concern,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sending the original record value to the MeetingCreatedEvtMsg builder and only overriding the RunningMeeting record. Tested with the HTML5 client and the interface matched the flag. Does the Flash client manages this differently? The messaging that flows after the MeetingCreatedEvtMsg can change this value or get it from RunningMeeting to feedback the clients?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I agree this is a little confusing. And I believe my test worked fine in the HTML5 client because Meteor saves everything in mongo... I'll make some changes. Thanks @ritzalam

@ritzalam
Copy link
Member

I am good with the changes on akka-apps. Will wait for @kepstin to give a go ahead for merging.

@kepstin
Copy link
Contributor

kepstin commented Jan 18, 2019

It looks like it's possible for a meeting to have both a "recorded" and an "ended" .done files created - in this case, the archiving will break. You need to ensure that the keep_events script and the archive script are never both run for the same meeting.

@pedrobmarin
Copy link
Collaborator Author

I believe I tested this use case, but you might be right. I'll make some extra modifications in this pull request next week

@@ -513,6 +518,7 @@ private void processRemoveEndedMeeting(MeetingEnded message) {
if (m != null) {
m.setForciblyEnded(true);
processRecording(m);
if (keepEvents) recordingService.markAsEnded(m.getInternalId());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kepstin here is what we discussed about writing recorded tag before the ended tag. markAsEnded will generate the ended tag after the processRecording method. processRecording will call startIngestAndProcessing and generate the recorded tag. So we should be good to go.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you just add a comment here to remark why the order is this way? Then everything should be good to go.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@ffdixon ffdixon merged commit 566b245 into bigbluebutton:master Jan 25, 2019
@pedrobmarin pedrobmarin deleted the keep-meeting-events branch November 2, 2019 19:06
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

Successfully merging this pull request may close these issues.

None yet

4 participants