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
Consolidate logging #19
Changes from 6 commits
5b2f8cc
46a5f5a
2b62a31
3509b9e
e1bbfd6
1a104d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,7 +122,7 @@ + (instancetype)sharedInstance { | |
sharedInstance = [[MultiPartyLoggingWrapper alloc] init]; | ||
sharedInstance.logger = [[OTKLogger alloc] initWithClientVersion:KLogClientVersion | ||
source:[[NSBundle mainBundle] bundleIdentifier] | ||
componentId:kLogComponentIdentifier | ||
componentId:KLogComponentIdentifier | ||
guid:[[NSUUID UUID] UUIDString]]; | ||
}); | ||
return sharedInstance; | ||
|
@@ -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]; | ||
} | ||
|
@@ -222,11 +213,25 @@ - (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) { | ||
|
||
if (self.screenSharingView) { | ||
[loggingWrapper.logger logEventAction:KLogActionStopScreensharing | ||
variation:KLogVariationAttempt | ||
completion:nil]; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Under screen sharing, it will log both. Should we put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Lucashuang0802 in Android, we are following the approach below:
So the stopPublishing log should not be in the else, it is covering the ss case as well. wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
I will add the modification to android core as well. thanks!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
else { | ||
[loggingWrapper.logger logEventAction:KLogActionStopPublishing | ||
variation:KLogVariationAttempt | ||
completion:nil]; | ||
} | ||
OTError *error = nil; | ||
[self.publisher.view removeFromSuperview]; | ||
[self.session unpublish:self.publisher error:&error]; | ||
|
@@ -242,29 +247,40 @@ - (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); | ||
} | ||
else { | ||
[loggingWrapper.logger logEventAction:KLogActionRemoveRemote | ||
variation:KLogVariationSuccess | ||
completion:nil]; | ||
} | ||
[subscriberObject.subscriberView removeFromSuperview]; | ||
[subscriberObject.subscriberView clean]; | ||
subscriberObject.subscriber = nil; | ||
subscriberObject.subscriberView = nil; | ||
|
||
} | ||
[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]; | ||
} | ||
|
@@ -295,23 +311,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) { | ||
if (!self.screenSharingView) { | ||
[loggingWrapper.logger logEventAction:KLogActionStartPublishing | ||
variation:KLogVariationAttempt | ||
completion:nil]; | ||
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; | ||
|
@@ -323,6 +347,16 @@ -(void)sessionDidConnect:(OTSession*)session { | |
OTError *error; | ||
[self.session publish:self.publisher error:&error]; | ||
if (error) { | ||
if (self.screenSharingView) { | ||
[loggingWrapper.logger logEventAction:KLogActionStartScreensharing | ||
variation:KLogVariationFailure | ||
completion:nil]; | ||
} | ||
else { | ||
[loggingWrapper.logger logEventAction:KLogActionStartPublishing | ||
variation:KLogVariationFailure | ||
completion:nil]; | ||
} | ||
[self notifyAllWithSignal:OTCommunicationError | ||
subscriber:nil | ||
error:error]; | ||
|
@@ -340,9 +374,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]; | ||
|
@@ -358,6 +397,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]; | ||
|
@@ -418,12 +460,41 @@ - (void)publisher:(OTPublisherKit *)publisher didFailWithError:(OTError *)error | |
} | ||
} | ||
|
||
- (void)publisher:(nonnull OTPublisherKit *)publisher streamCreated:(nonnull OTStream *)stream { | ||
if (stream.videoType == OTStreamVideoTypeScreen ) { | ||
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStartScreensharing | ||
variation:KLogVariationSuccess | ||
completion:nil]; | ||
} | ||
else { | ||
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStartPublishing | ||
variation:KLogVariationSuccess | ||
completion:nil]; | ||
} | ||
} | ||
|
||
- (void)publisher:(nonnull OTPublisherKit *)publisher streamDestroyed:(nonnull OTStream *)stream { | ||
if (stream.videoType == OTStreamVideoTypeScreen ) { | ||
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStopScreensharing | ||
variation:KLogVariationSuccess | ||
completion:nil]; | ||
} | ||
else { | ||
[[MultiPartyLoggingWrapper sharedInstance].logger logEventAction:KLogActionStopPublishing | ||
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; | ||
} | ||
} | ||
|
@@ -524,32 +595,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]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For cases like this, why do we even have |
||
} | ||
|
||
- (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 | ||
|
@@ -580,8 +683,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]; | ||
|
There was a problem hiding this comment.
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?