-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[ADDED] Making monitoring endpoints available via system services. #1362
Conversation
This is nice, but these end points over http have many more options. Should we consider accepting a JSON blob of options? |
We should support the options for sure, could be generic string that is the url args? REQ .CONNZ subs=1&sort=id |
@ripienaar @derekcollison are there options not covered by the options struct? "VARZ": func(sub *subscription, _ *client, subject, reply string, msg []byte) {
optz := &VarzOptions{}
s.zReq(reply, msg, optz, func() (interface{}, error) { return s.Varz(optz) })
},
Example with content:
|
Ah I missed you already have JSON payload there. Then it’s good. Sorry.l |
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.
LGTM!
changed error return to include http status and thus match jetstream api ADR
|
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.
Seem you set status to internal error regardless of err
value.
} | ||
if err == nil { | ||
response["data"], err = respf() | ||
status = http.StatusInternalServerError |
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 seem weird to me. You set it regardless of the err
result.
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.
because status is only returned when there is an error, I can basically set it all the time essentially keeping track of how far I got. But will change
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.
Right, hence comment above when doing the unmarshal.
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.
LGTM
Available via $SYS.REQ.SERVER.%s.%s and $SYS.REQ.SERVER.PING.%s Last token is the endpoint name. Signed-off-by: Matthias Hanel <mh@synadia.com>
5da50a3
to
14c7160
Compare
@derekcollison, go / no-go / defer to Ivan? |
I will defer to @kozlovic. Thanks for ping. |
I am exposing these as there should not be two different code paths and that enhancements to one are available in the other as well.
I'm also establishing this as the pattern so that we can inspect server state via http as well as nats.
Available via $SYS.REQ.SERVER.%s.%s and $SYS.REQ.SERVER.PING.%s
Last token is the endpoint name.
Since this is in system events I went for less code and thus use of closures over avoiding them
This can change if you want to.
Input can be empty or a json of the corresponding options.
return always includes server. error and data are mutually exclusive.
I can rename data to smth. more specific (connz/subsz/...) as well.
connz and subz have a limit option. Limit and offset are honored by this.
We can consider adding code to return a chunked response.
But I would only do that in addition to and not instead of.
existing tooling such as nats top should be easily adaptable.
We should also consider adding cluster subjects.
fmt.Sprintf("$SYS.REQ.CLUSTER.%s.PING.%s", s.info.Cluster, name)
Such that we can query one particular cluster by name.
The cluster name is only set when using gateways.
Maybe we should allow it to be specified even when gateways are not used, so that a request can refer to the same group of server. Opinions?
Signed-off-by: Matthias Hanel mh@synadia.com
Sending a request for each type. (ping and by server id)