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.

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

Enhance Remote Monitor Experience with JSON logging, Special endpoint for Twilio & Message Logging #3178

Open
mastercactapus opened this issue Jul 17, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@mastercactapus
Copy link
Member

What problem would you like to solve? Please describe:
The current remote monitor module lacks optimized functionalities such as JSON logging, a dedicated endpoint for Twilio calls and a mechanism to trace messages back to their source.

Describe the solution you'd like:

  1. Implement JSON logging for the remote monitor, ensuring alignment with the rest of the application.
  2. Create a specific endpoint for Twilio calls to avoid 'Missing X-Twilio-Signature' errors. This will also reduce confusion by separating expected Twilio traffic from other requests.
  3. Log the message SIDs for better traceability. This will allow us to trace messages through logs elsewhere, facilitating easier troubleshooting and monitoring.

Describe alternatives you've considered:
Currently, it's possible to conduct a track-back of issues to Twilio using time estimates and educated guesses, or running tcpdump on the pod. However, these methods are inefficient and imprecise.

Additional context:
The proposed features will enhance the user experience for remote monitor by providing better logging, error reduction, and traceability of messages.

@mastercactapus mastercactapus added the enhancement New feature or request label Jul 17, 2023
@kingledion
Copy link

Can you explain more details about

Create a specific endpoint for Twilio calls to avoid 'Missing X-Twilio-Signature' errors. This will also reduce confusion by separating expected Twilio traffic from other requests.

@1ddo
Copy link
Contributor

1ddo commented Jul 27, 2023

for #2 request, upon checking the code, the error originates from a validation function and it seems like this validation is only related to Twilio requests based on the error messages it's throwing. So I would like to ask if this validation should only be applied to Twilio requests and exclude all other requests from it, that way unrelated requests would not throw this Twilio error, is that right?

Checking the code app/inithttp.go, starting from line 190, the validation only filters for Twilio requests so I think there's no need for #2. Just need someone to double check it.

For #3, the implemented logging (util/log/log.go) already integrates the RequestID in all log levels. I just don't know if RequestID is same as the SIDs...

1ddo pushed a commit to 1ddo/goalert that referenced this issue Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants