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.

Sign up for GitHub

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

Jump to bottom

Consolidate logging #19

Closed
wants to merge 6 commits into from
Closed

Consolidate logging #19

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5b2f8cc
Fixing client logs
marinaserranomontes Feb 15, 2017
46a5f5a
Fixing logging
marinaserranomontes Feb 15, 2017
2b62a31
Cleaning
marinaserranomontes Feb 15, 2017
3509b9e
Fixing PR comments
marinaserranomontes Feb 16, 2017
e1bbfd6
fixing logging for start/stop publishing
marinaserranomontes Feb 20, 2017
1a104d7
Update branch
marinaserranomontes Feb 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 15 additions & 5 deletions OTAcceleratorCore/OTCommonCommunicator.h
View file
Open in desktop
Expand Up @@ -8,12 +8,22 @@

#import <Foundation/Foundation.h>

static NSString* const KLogClientVersion = @"ios-vsol-2.0.0";
static NSString* const kLogComponentIdentifier = @"OTAcceleratorCore";
static NSString* const KLogClientVersion = @"ios-vsol-1.0.4";
static NSString* const kLogComponentIdentifier = @"acceleratorCore";
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use upper case K for making it consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

static NSString* const KLogActionInitialize = @"Init";
static NSString* const KLogActionStartCommunication = @"StartComm";
static NSString* const KLogActionStartScreenCommunication = @"StartScreenComm";
static NSString* const KLogActionEndCommunication = @"EndComm";
static NSString* const KLogActionConnect = @"Connect";
static NSString* const KLogActionDisconnect = @"Disconnect";
static NSString* const KLogActionStartPublishing = @"StartPublishingMedia";
static NSString* const KLogActionStopPublishing = @"StopPublishingMedia";
static NSString* const KLogActionStartScreensharing = @"StartScreensharing";
static NSString* const KLogActionStopScreensharing = @"StopScreensharing";
static NSString* const KLogActionAddRemote = @"AddRemote";
static NSString* const KLogActionRemoveRemote = @"RemoveRemote";
static NSString* const KLogActionIsLocalMediaEnabled = @"IsLocalMediaEnabled";
static NSString* const KLogActionEnableLocalMedia = @"EnableLocalMedia";
static NSString* const KLogActionIsReceivedMediaEnabled = @"IsReceivedMediaEnabled";
static NSString* const KLogActionEnableReceivedMedia = @"EnableReceivedMedia";
static NSString* const KLogActionCycleCamera = @"CycleCamera";
static NSString* const KLogVariationAttempt = @"Attempt";
static NSString* const KLogVariationSuccess = @"Success";
static NSString* const KLogVariationFailure = @"Failure";
Expand Down
148 changes: 119 additions & 29 deletions OTAcceleratorCore/OTMultiPartyCommunicator.m
View file
Open in desktop
Expand Up @@ -192,28 +192,19 @@ - (instancetype)initWithView:(UIView *)view name:(NSString *)name {
}

- (NSError *)connect {

MultiPartyLoggingWrapper *loggingWrapper = [MultiPartyLoggingWrapper sharedInstance];
if (!_screenSharingView) {
[loggingWrapper.logger logEventAction:KLogActionStartCommunication
variation:KLogVariationAttempt
completion:nil];
}
else {
[loggingWrapper.logger logEventAction:KLogActionStartScreenCommunication
variation:KLogVariationAttempt
completion:nil];
}

[loggingWrapper.logger logEventAction:KLogActionConnect
variation:KLogVariationAttempt
completion:nil];

NSError *connectError = [self.session registerWithAccePack:self];
if (!connectError) {
[loggingWrapper.logger logEventAction:KLogActionStartCommunication
[loggingWrapper.logger logEventAction:KLogActionConnect
variation:KLogVariationSuccess
completion:nil];
}
else {
[loggingWrapper.logger logEventAction:KLogActionStartCommunication
[loggingWrapper.logger logEventAction:KLogActionConnect
variation:KLogVariationFailure
completion:nil];
}
Expand All @@ -222,11 +213,23 @@ - (NSError *)connect {
}

- (NSError *)disconnect {
MultiPartyLoggingWrapper *loggingWrapper = [MultiPartyLoggingWrapper sharedInstance];

[loggingWrapper.logger logEventAction:KLogActionDisconnect
variation:KLogVariationAttempt
completion:nil];

// need to explicitly unpublish and unsubscriber if the communicator is the only accelerator to dismiss from the common session
// when there are multiple accelerator packs, the common session will not call the disconnect method until the last delegate object is removed
if (self.publisher) {

[loggingWrapper.logger logEventAction:KLogActionStopPublishing
variation:KLogVariationAttempt
completion:nil];
if (self.screenSharingView) {
[loggingWrapper.logger logEventAction:KLogActionStopScreensharing
variation:KLogVariationAttempt
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it the attempt action here?

completion:nil];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Under screen sharing, it will log both. Should we put KLogActionStopPublishing to the else branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Lucashuang0802 in Android, we are following the approach below:

  • start/stop publishing means starting to share your media independently on if it is screen or not.
  • start/stop screensharing means you are publishing and also, it is the screensharing case.

So the stopPublishing log should not be in the else, it is covering the ss case as well. wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

It does not make too much sense to me as it's the same event but different capture type. and I do believe publishing and screen-sharing are same but capturing from a different feed. that will probably create confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After review it, I agree. Maybe, it could be confused, so, we are going to log:

  • start/stop publishing media (camera stream)
  • start/stop screensharing

I will add the modification to android core as well. thanks!!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

OTError *error = nil;
[self.publisher.view removeFromSuperview];
[self.session unpublish:self.publisher error:&error];
Expand All @@ -242,29 +245,37 @@ - (NSError *)disconnect {
}

for (OTMultiPartyRemote *subscriberObject in self.subscribers) {
[loggingWrapper.logger logEventAction:KLogActionRemoveRemote
variation:KLogVariationAttempt
completion:nil];
OTError *error = nil;
OTSubscriber *subscriber = subscriberObject.subscriber;
[subscriber.view removeFromSuperview];
[self.session unsubscribe:subscriber error:&error];
if (error) {
[loggingWrapper.logger logEventAction:KLogActionRemoveRemote
variation:KLogVariationFailure
completion:nil];
NSLog(@"%s: %@", __PRETTY_FUNCTION__, error);
}
[subscriberObject.subscriberView removeFromSuperview];
[subscriberObject.subscriberView clean];
subscriberObject.subscriber = nil;
subscriberObject.subscriberView = nil;
[loggingWrapper.logger logEventAction:KLogActionRemoveRemote
variation:KLogVariationSuccess
completion:nil];
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, we will have a situation to log success and failure the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Lucashuang0802 you are right, the error is not a return. I will add the success case in the else.

}
[self.subscribers removeAllObjects];

MultiPartyLoggingWrapper *loggingWrapper = [MultiPartyLoggingWrapper sharedInstance];
NSError *disconnectError = [self.session deregisterWithAccePack:self];
if (!disconnectError) {
[loggingWrapper.logger logEventAction:KLogActionEndCommunication
[loggingWrapper.logger logEventAction:KLogActionDisconnect
variation:KLogVariationSuccess
completion:nil];
}
else {
[loggingWrapper.logger logEventAction:KLogActionEndCommunication
[loggingWrapper.logger logEventAction:KLogActionDisconnect
variation:KLogVariationFailure
completion:nil];
}
Expand Down Expand Up @@ -295,23 +306,31 @@ - (void)notifyAllWithSignal:(OTCommunicationSignal)signal

#pragma mark - OTSessionDelegate
-(void)sessionDidConnect:(OTSession*)session {

[[MultiPartyLoggingWrapper sharedInstance].logger setSessionId:session.sessionId
connectionId:session.connection.connectionId
partnerId:@([self.session.apiKey integerValue])];
MultiPartyLoggingWrapper *loggingWrapper = [MultiPartyLoggingWrapper sharedInstance];
[loggingWrapper.logger setSessionId:session.sessionId
connectionId:session.connection.connectionId
partnerId:@([self.session.apiKey integerValue])];
[loggingWrapper.logger logEventAction:KLogActionConnect
variation:KLogVariationSuccess
completion:nil];

if (!self.publisher) {
[loggingWrapper.logger logEventAction:KLogActionStartPublishing
variation:KLogVariationAttempt
completion:nil];
if (!self.screenSharingView) {
OTPublisherSettings *setting = [[OTPublisherSettings alloc] init];
setting.name = self.name;
self.publisher = [[OTPublisher alloc] initWithDelegate:self settings:setting];
}
else {
OTPublisherSettings *setting = [[OTPublisherSettings alloc] init];
setting.name = self.name;
setting.audioTrack = YES;
setting.videoTrack = YES;
self.publisher = [[OTPublisher alloc] initWithDelegate:self settings:setting];
[loggingWrapper.logger logEventAction:KLogActionStartScreensharing
variation:KLogVariationAttempt
completion:nil];
self.publisher = [[OTPublisher alloc] initWithDelegate:self
name:self.name
audioTrack:YES
videoTrack:YES];

[self.publisher setVideoType:OTPublisherKitVideoTypeScreen];
self.publisher.audioFallbackEnabled = NO;
Expand All @@ -323,6 +342,14 @@ -(void)sessionDidConnect:(OTSession*)session {
OTError *error;
[self.session publish:self.publisher error:&error];
if (error) {
[loggingWrapper.logger logEventAction:KLogActionStartPublishing
variation:KLogVariationFailure
completion:nil];
if (self.screenSharingView) {
[loggingWrapper.logger logEventAction:KLogActionStartScreensharing
variation:KLogVariationFailure
completion:nil];
}
[self notifyAllWithSignal:OTCommunicationError
subscriber:nil
error:error];
Expand All @@ -340,9 +367,14 @@ -(void)sessionDidConnect:(OTSession*)session {
}

- (void)session:(OTSession *)session streamCreated:(OTStream *)stream {
MultiPartyLoggingWrapper *loggingWrapper = [MultiPartyLoggingWrapper sharedInstance];

if (self.isPublishOnly) return;

[loggingWrapper.logger logEventAction:KLogActionAddRemote
variation:KLogVariationAttempt
completion:nil];

OTError *subscrciberError;
OTSubscriber *subscriber = [[OTSubscriber alloc] initWithStream:stream delegate:self];
[self.session subscribe:subscriber error:&subscrciberError];
Expand All @@ -358,6 +390,9 @@ - (void)session:(OTSession *)session streamCreated:(OTStream *)stream {
error:nil];
}
else {
[loggingWrapper.logger logEventAction:KLogActionAddRemote
variation:KLogVariationFailure
completion:nil];
[self notifyAllWithSignal:OTCommunicationError
subscriber:nil
error:subscrciberError];
Expand Down Expand Up @@ -418,12 +453,37 @@ - (void)publisher:(OTPublisherKit *)publisher didFailWithError:(OTError *)error
}
}

- (void)publisher:(nonnull OTPublisherKit *)publisher streamCreated:(nonnull OTStream *)stream {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStartPublishing
variation:KLogVariationSuccess
completion:nil];
if (stream.videoType == OTStreamVideoTypeScreen ) {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStartScreensharing
variation:KLogVariationSuccess
completion:nil];
}
}

- (void)publisher:(nonnull OTPublisherKit *)publisher streamDestroyed:(nonnull OTStream *)stream {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStopPublishing
variation:KLogVariationSuccess
completion:nil];
if (stream.videoType == OTStreamVideoTypeScreen ) {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStopScreensharing
variation:KLogVariationSuccess
completion:nil];
}
}

- (void)subscriberDidConnectToStream:(OTSubscriber *)subscriber {
for (OTMultiPartyRemote *subscriberObject in self.subscribers) {
if (subscriberObject.subscriber == subscriber) {
[self notifyAllWithSignal:OTSubscriberReady
subscriber:subscriberObject
error:nil];
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionAddRemote
variation:KLogVariationSuccess
completion:nil];
break;
}
}
Expand Down Expand Up @@ -524,32 +584,64 @@ - (void)placeHolderImageViewDidDismissOnVideoView:(OTVideoView *)videoView {
}

- (void)setPublishAudio:(BOOL)publishAudio {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionEnableLocalMedia
variation:KLogVariationAttempt
completion:nil];
if (!_publisher) return;
_publisher.publishAudio = publishAudio;
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionEnableLocalMedia
variation:KLogVariationSuccess
completion:nil];
Copy link
Contributor

Choose a reason for hiding this comment

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

For cases like this, why do we even have attempt if we don't have a case to fail?

}

- (BOOL)isPublishAudio {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionIsLocalMediaEnabled
variation:KLogVariationAttempt
completion:nil];
if (!_publisher) return NO;
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionIsLocalMediaEnabled
variation:KLogVariationSuccess
completion:nil];
return _publisher.publishAudio;
}


- (void)setPublishVideo:(BOOL)publishVideo {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionEnableLocalMedia
variation:KLogVariationAttempt
completion:nil];
if (!_publisher) return;
_publisher.publishVideo = publishVideo;
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionEnableLocalMedia
variation:KLogVariationSuccess
completion:nil];
}

- (BOOL)isPublishVideo {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionIsLocalMediaEnabled
variation:KLogVariationAttempt
completion:nil];
if (!_publisher) return NO;
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionIsLocalMediaEnabled
variation:KLogVariationSuccess
completion:nil];
return _publisher.publishVideo;
}


- (AVCaptureDevicePosition)cameraPosition {
return _publisher.cameraPosition;
}

- (void)setCameraPosition:(AVCaptureDevicePosition)cameraPosition {
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionCycleCamera
variation:KLogVariationAttempt
completion:nil];
if (self.screenSharingView) return;
_publisher.cameraPosition = cameraPosition;
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionCycleCamera
variation:KLogVariationSuccess
completion:nil];
}

#pragma mark PublishOnly flag
Expand Down Expand Up @@ -580,8 +672,6 @@ - (void)updateSubscriber {
else {
NSDictionary *streams = self.session.streams;
for (OTStream *stream in streams.allValues) {


if (stream.connection != self.session.connection) {
OTError *subscriberError;
OTSubscriber *subscriber = [[OTSubscriber alloc] initWithStream:stream delegate:self];
Expand Down