-
Notifications
You must be signed in to change notification settings - Fork 76
Adding a custom struct to use for call end hooks #396
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
Adding a custom struct to use for call end hooks #396
Conversation
The struct has project id, the scl call id generated by livekit/livekit and the protocol level sip call id
bf9c2e8
to
71ac631
Compare
…ng call drop use cases before the connection was established
pkg/sip/outbound.go
Outdated
go c.c.handler.OnSessionEnd(context.Background(), &CallIdentifier{ | ||
ProjectID: c.projectID, | ||
CallID: c.state.callInfo.CallId, | ||
SipCallID: c.state.callInfo.ParticipantAttributes[AttrSIPCallIDFull], |
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.
It should be available as a field in the call itself. If not, feel free to add it. Please avoid getting things from attribute map.
pkg/sip/inbound.go
Outdated
@@ -827,7 +851,7 @@ func (c *inboundCall) close(error bool, status CallStatus, reason string) { | |||
|
|||
// Call the handler asynchronously to avoid blocking | |||
if c.s.handler != nil { | |||
go c.s.handler.OnCallEnd(context.Background(), c.state.callInfo, reason) | |||
c.s.handleSessionEnd(context.Background(), c.call, c.state, c.state.callInfo.ParticipantAttributes[AttrSIPCallIDFull], reason, c.projectID) |
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.
Same note for the call ID as below: it should be a field on a call, getting things from attributes should be avoided.
pkg/sip/inbound.go
Outdated
if h := req.CallID(); h != nil { | ||
sipCallID = h.Value() | ||
} | ||
s.handleSessionEnd(ctx, callInfo, state, sipCallID, "flood", r.ProjectID) |
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.
This might trigger very frequently. And we won't have a projectID here, so I don't think it's useful, since we can't surface it in the dashboard.
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.
Only issue with not having this here is it would lead to a bunch of pcaps not getting cleaned out. There is logic to upload pcaps under a missingprojects folder if we can't identify the project. Afaict, it will not get triggered more than once per SIP session. Are you saying that it's much higher scale vs that of call connections ?
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.
I'll remove this for now. But we might need to revisit in case those unconnected calls are too many and we have to deal with secondary memory leakage. We've got 10GB spare so I doubt it will be a problem.
pkg/service/service.go
Outdated
func (s *Service) OnCallEnd(ctx context.Context, callInfo *livekit.SIPCallInfo, reason string) { | ||
s.log.Infow("SIP call ended", "callID", callInfo.CallId, "reason", reason) | ||
func (s *Service) OnSessionEnd(ctx context.Context, callIdentifier *sip.CallIdentifier, callInfo *livekit.SIPCallInfo, reason string) { | ||
s.log.Infow("Post SIP call end processing", "callID", callInfo.CallId, "reason", reason) |
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.
No actual processing here. So maybe just keep "call ended" log?
* adding a custom struct to use for call end hooks The struct has project id, the scl call id generated by livekit/livekit and the protocol level sip call id * Assigning the right values to the identifiers * Improving the tests a bit * Correctly passing the ProjectID at the end of the call * Using Session End instead of CallEnd to be more accurate. Also handling call drop use cases before the connection was established * Ensuring project id is available prior to call connection * Changes from PR feedback * Pulling in latest version of protocol to add SipCallId to SipCall
The struct has project id, the scl call id generated by livekit/livekit and the protocol level sip call id