Add Support for Twilio Copilot

[cdmastercom]

Per Twilio Docs add support for alerts using Copilot. Compability with the existing Twilio SMS alerting method is maintained.

• Make 'twilio_from_number' optional
• Add setting 'twilio_message_service_sid'
• Add setting 'twilio_use_copilot', defaults to False

In case 'twilio_use_copilot' is False, 'twilio_from_number' is required. Otherwise, 'twilio_message_service_sid' is required.

I've tested this using an Elasticsearch 7.1.1 cluster, both SMS and Copilot alerting work.

Thanks to @nsano-rururu for letting me know that the yelp repository was abandoned.

[nsano-rururu]

@cdmastercom

Wouldn't it be necessary to add to the documentation that twilio_from_number is no longer required, that twilio_message_service_sid and twilio_use_copilot have been added as an option, and that the default value for twilio_use_copilot is "False"?
https://github.com/jertel/elastalert2/blob/master/docs/source/ruletypes.rst#twilio

[cdmastercom]

No problem @nsano-rururu, how does this look?

[nsano-rururu]

@cdmastercom

Is it the following setting image?

For SMS

alert:
- "twilio"
twilio_account_sid: "xxxxxxx"
twilio_auth_token: "xxxxxxx"
twilio_to_number: "xxxxxxx"
twilio_from_number: "xxxxxxx"

For Copilot

alert:
- "twilio"
twilio_use_copilot: True
twilio_account_sid: "xxxxxxx"
twilio_auth_token: "xxxxxxx"
twilio_to_number: "xxxxxxx"
twilio_message_service_sid: "xxxxxxx"

Don't you add the two settings added to schema.yaml? .. If the type is different when executing the rule, an error will occur.

https://github.com/jertel/elastalert2/blob/master/elastalert/schema.yaml#L325

twilio_message_service_sid: {type: string}
twilio_use_copilot: {type: boolean}

[cdmastercom]

Alright, added that now @nsano-rururu. Sorry I'm not too familiar with this project.

Is there anything else I missed?

[cdmastercom]

Yes the way you wrote the settings is correct. Would it be better if this was split into two different Alerter classes?

[nsano-rururu]

@cdmastercom

Yes the way you wrote the settings is correct. Would it be better if this was split into two different Alerter classes?

I think you can leave it as it is. I think it would be nice to write two setting examples as described in The Hive's documentation.
https://github.com/jertel/elastalert2/blob/master/docs/source/ruletypes.rst#thehive

[nsano-rururu]

@cdmastercom

"Twilio>= 6.0.0,<6.1" is specified in setup.py and requirements.txt, but if you change it to "twilio>=6.0.0,<6.58", ask them to try it. Is it possible?

I want to know if there is no problem with updating the library or if it should be the same version.

[nsano-rururu]

@cdmastercom

If that is not possible, there is no problem if you answer that it is impossible.

[cdmastercom]

@nsano-rururu

That worked. Pip was able to install the modified requirements, and SMS still worked as expected. The twilio module API for sending sms doesn't appear to have changed, so I think we can support the whole range of versions from 6.0 to 6.57.

[nsano-rururu]

@jertel

My review is over.
Is there anything else I can point out?

[jertel]

@jertel

My review is over.
Is there anything else I can point out?

No, go ahead and approve and merge it. Thanks!

[ferozsalam]

@cdmastercom Could you also add a line to the changelog under the list of new features please?

[jertel]

I'll update the changelog in a few mins.

[cdmastercom]

Thanks guys.