-
Notifications
You must be signed in to change notification settings - Fork 97
Adding support for dynamic dispatch rules #1123
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
base: main
Are you sure you want to change the base?
Conversation
nishadmusthafa
commented
Jun 30, 2025
- Adding the capability for dispatch rule to have a url and http method
- Allowing the user to respond with a dynamic response for the url
- Allowing three types of dispatch actions(answer, hangup and transfer)
1. Adding the capability for dispatch rule to have a url and http method 2. Allowing the user to respond with a dynamic response for the url 3. Allowing three types of dispatch actions(answer, hangup and transfer)
🦋 Changeset detectedLatest commit: c71f955 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR 💥 An error occurred when fetching the changed packages and changesets in this PR
|
PR for dynamic dispatch rules: livekit/protocol#1123 go.mod changes will be made once that PR is approved.
This is the cli change -> livekit/livekit-cli#606 |
if dynamicRule.Url == "" { | ||
return errors.New("dynamic dispatch rule URL cannot be empty") | ||
} | ||
if !strings.HasPrefix(dynamicRule.Url, "https://") { |
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.
Plain HTTP should probably be allowed for OSS.
// Type of action to take | ||
DispatchAction action = 1; | ||
|
||
// Fields used when action is ACTION_ANSWER |
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 should be a separate message and a oneof
field. In that case we no longer need a DispatchAction
enum.
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.
hah.. that's exactly what I'd done earlier. But @davidzhao and I had a discussion on this. My initial version had a bunch of nesting and using messages in messages. The idea here was to make it simple and give users the ability to use as flat a structure as possible when returning a response.
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.
Just to add more context here, the flat json for response as well as for payload is so that anyone who wants to use this doesn't need to have the sdk to be able to do this.
string room_preset = 4; | ||
// RoomConfiguration to use if the participant initiates the room | ||
RoomConfiguration room_config = 5; | ||
//valid for only cloud | ||
bool krisp_enabled = 6; | ||
SIPMediaEncryption media_encryption = 7; |
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.
We should probably put these common fields in some message type and share in for inbound and outbound calls. Could be done later, of course.
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 point on flat json as above
// Room name to connect to | ||
string room_name = 2; | ||
// Optional pin required to enter room | ||
string pin = 3; |
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 believe this would change the dispatch logic significantly. I wonder if an entered pin should be sent as a new webhook event.
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.
Yeah, DZ was also mentioning webhooks for another category of events. This would be if the agent fails to connect. Are we talking about adding more events here -> https://github.com/livekit/protocol/blob/main/webhook/consts.go#L27 ?
|
||
// Fields from SIPDispatchRuleDynamicResponse for dynamic rules | ||
// Type of action to take | ||
DispatchAction action = 13; |
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.
Shouldn't it already be in the rule
field?
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.
Not really, action is only in the SIPDispatchRuleDynamicResponse and only when it's a dynamic rule. So we have to explicitly set it once a response comes back.
@@ -810,6 +822,8 @@ func MatchDispatchRuleIter(trunk *livekit.SIPInboundTrunkInfo, rules iters.Iter[ | |||
|
|||
// EvaluateDispatchRule checks a selected Dispatch Rule against the provided request. | |||
func EvaluateDispatchRule(projectID string, trunk *livekit.SIPInboundTrunkInfo, rule *livekit.SIPDispatchRuleInfo, req *rpc.EvaluateSIPDispatchRulesRequest) (*rpc.EvaluateSIPDispatchRulesResponse, error) { | |||
log := logger.GetLogger() | |||
log.Debugw("Evaluating dispatch rule dailamooooo", "rule", rule) |
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.
Should be removed?
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.
Yes 😅. thanks for catching that.
return nil, twirp.WrapError(twirp.NewError(twirp.Internal, "Failed to get room and pin"), err) | ||
} | ||
case *livekit.SIPDispatchRule_DispatchRuleDynamic: | ||
type dynamicDispatchRequest struct { |
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.
We should define these as protobufs and use protojson to encode/decode.
return nil, twirp.WrapError(twirp.NewError(twirp.Internal, "Failed to marshal request"), err) | ||
} | ||
|
||
resp, err := http.Post(dynamicRule.DispatchRuleDynamic.GetUrl(), "application/json", bytes.NewReader(reqBytes)) |
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.
We might want an option to pass a custom http.Client
here.
if err != nil { | ||
return nil, twirp.WrapError(twirp.NewError(twirp.Internal, "Failed to get room and pin"), err) | ||
} | ||
case *livekit.SIPDispatchRule_DispatchRuleDynamic: |
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.
Worth moving to a separate function.