-
-
Notifications
You must be signed in to change notification settings - Fork 428
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
feat: support monitoring WebSocket endpoints (wss) #511
Conversation
2c8af67
to
ac2b573
Compare
WebSocket endpoints are automatically identified by the URL protocol specification: `wss://` or `ws://`. The request body is used as the "message" written to the server, and the answer is stored in the `[BODY]`. Optionally, the user can set the `jsonrpc` flag to automatically wrap the request body in a JSON RPC 2.0 method call.
ac2b573
to
33994a9
Compare
- test we can identify the endpoint type for WebSockets based on the URL supplied: `wss://` (with SSL/TLS) and `ws://` (plain text). - test we can generate a JsonRPC 2.0 message via the new endpoint flag `JsonRPC`.
Current tests: $ make test
go test ./... -cover
? github.com/TwiN/gatus/v5 [no test files]
? github.com/TwiN/gatus/v5/alerting [no test files]
ok github.com/TwiN/gatus/v5/alerting/alert (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/alerting/provider (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/custom (cached) coverage: 95.1% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/discord (cached) coverage: 95.5% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/email (cached) coverage: 70.0% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/github (cached) coverage: 82.4% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/gitlab (cached) coverage: 85.7% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/googlechat (cached) coverage: 95.9% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/matrix (cached) coverage: 97.0% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/mattermost (cached) coverage: 95.6% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/messagebird (cached) coverage: 90.9% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/ntfy (cached) coverage: 51.7% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/opsgenie (cached) coverage: 90.0% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/pagerduty (cached) coverage: 79.5% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/pushover (cached) coverage: 92.3% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/slack (cached) coverage: 95.3% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/teams (cached) coverage: 95.5% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/telegram (cached) coverage: 91.7% of statements
ok github.com/TwiN/gatus/v5/alerting/provider/twilio (cached) coverage: 33.3% of statements
? github.com/TwiN/gatus/v5/config/remote [no test files]
? github.com/TwiN/gatus/v5/core/ui [no test files]
? github.com/TwiN/gatus/v5/storage [no test files]
? github.com/TwiN/gatus/v5/storage/store/common [no test files]
? github.com/TwiN/gatus/v5/test [no test files]
--- FAIL: TestPing (0.00s)
client_test.go:69: expected true
client_test.go:71: Round-trip time returned on success should've higher than 0
FAIL
github.com/TwiN/gatus/v5/client coverage: 67.4% of statements
FAIL github.com/TwiN/gatus/v5/client 3.967s
ok github.com/TwiN/gatus/v5/config (cached) coverage: 83.9% of statements
ok github.com/TwiN/gatus/v5/config/connectivity (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/config/maintenance (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/config/ui (cached) coverage: 78.3% of statements
ok github.com/TwiN/gatus/v5/config/web (cached) coverage: 81.8% of statements
ok github.com/TwiN/gatus/v5/controller (cached) coverage: 78.6% of statements
ok github.com/TwiN/gatus/v5/controller/handler (cached) coverage: 78.9% of statements
--- FAIL: TestIntegrationEvaluateHealthForICMP (0.00s)
endpoint_test.go:741: Conditions '[CONNECTED] == true' should have been a success
endpoint_test.go:744: Because the connection has been established, result.Connected should've been true
endpoint_test.go:747: Because all conditions passed, this should have been a success
FAIL
github.com/TwiN/gatus/v5/core coverage: 85.9% of statements
FAIL github.com/TwiN/gatus/v5/core 2.838s
ok github.com/TwiN/gatus/v5/jsonpath (cached) coverage: 97.1% of statements
ok github.com/TwiN/gatus/v5/metrics (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/pattern (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/security (cached) coverage: 61.2% of statements
ok github.com/TwiN/gatus/v5/storage/store (cached) coverage: 93.5% of statements
ok github.com/TwiN/gatus/v5/storage/store/common/paging (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/storage/store/memory (cached) coverage: 88.8% of statements
ok github.com/TwiN/gatus/v5/storage/store/sql (cached) coverage: 81.6% of statements
ok github.com/TwiN/gatus/v5/util (cached) coverage: 100.0% of statements
ok github.com/TwiN/gatus/v5/watchdog (cached) coverage: 51.3% of statements
ok github.com/TwiN/gatus/v5/web (cached) coverage: [no statements]
FAIL
make: *** [Makefile:16: test] Error 1 Do you have any suggestions on how to test the new code in the |
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 forgot to submit the PR review 🤦♂️
A little late and some of the comments may no longer be relevant, but here goes
@heitorPB To answer your questions on the two failing tests about ping/icmp -- feel free to ignore them for now. They're flaky and they don't work on everybody's local environment. |
Move the `defer ws.Close()` to after opening the connection, so the socket is closed also in case of errors.
@TwiN Thank you for the feedback :) I addressed your points in the last commits. |
Btw, I was told that the websocket library used here is not maintained, and was suggested Gorrilla instead, which is/was also unmaintaned. What are your thoughts on this? Edit: found a maintained fork o Gorilla/websocket: https://github.com/fasthttp/websocket |
The connection should only be closed if successfully opened.
core/websocket.go
Outdated
package core | ||
|
||
import ( | ||
"golang.org/x/net/websocket" | ||
) | ||
|
||
func queryWebSocket(endpoint *Endpoint, result *Result) { | ||
const ( | ||
Origin = "http://localhost/" | ||
MaximumMessageSize = 1024 // in bytes | ||
) |
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'm a bit conflicted here, because historically, actual calls were made from the client
package here: https://github.com/TwiN/gatus/blob/master/client/client.go as opposed to the core package, but if you used the dns file as an example, I can see why you'd put it in the core package instead.
Still, could you move this functionality in the client package instead? I'll take care of the dns one separately to avoid future confusion
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, I used the DNS as example.
I can move the code to the client
package, no problem. Will move the function there and follow the same terminology. I should rename the function to CanCreateWebSocketConnection
?
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.
Also, do you want me to rebase this branch on top of current master
? I saw that you merged master
in here.
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.
Naming it QueryWebsocket
is fine; you'll have to modify the function's signature so that it doesn't pass core.Endpoint
and core.Result
though, just like the other functions in the client
package.
As for the rebase question- I'll squash when I merge anyways, so don't worry about it.
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 moved the function to the client package. Had to add other arguments and return to accomododate a body
and a return message. Hope it is fine, but if you know of a better way I can rework it.
And a noob question: how do I compile this branch?
$ go build -a -installsuffix cgo -o gatus .
# github.com/TwiN/gatus/v5/core
core/endpoint.go:347:47: undefined: client.queryWebSocket
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.
Oh, I needed to have the function in Uppercase.
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #511 +/- ##
==========================================
- Coverage 81.65% 80.88% -0.78%
==========================================
Files 56 57 +1
Lines 4350 4389 +39
==========================================
- Hits 3552 3550 -2
- Misses 617 655 +38
- Partials 181 184 +3
☔ View full report in Codecov by Sentry. |
@heitorPB Thank you for the contribution! |
Thank you for Gatus and the super valuable feedback you left here, @TwiN! I'll keep this in mind for the next PR :D |
Summary
Solves #492
Checklist
README.md
, if applicable.