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

api: Expose mesh status #644

Merged
merged 2 commits into from
Mar 7, 2017
Merged

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Mar 7, 2017

The weaveworks mesh package reveals information about the current status
of the mesh network between alertmanager instances. This commit exposes
the current address and connection status of each instance connected to
the targeted alertmanager instance via the /status API endpoint.

The weaveworks mesh package reveals information about the current status
of the mesh network between alertmanager instances. This commit exposes
the current address and connection status of each instance connected to
the targeted alertmanager instance via the /status API endpoint.
@mxinden
Copy link
Member Author

mxinden commented Mar 7, 2017

@fabxc Could you review this PR? What other status info would you like to see included as well?

Copy link
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good! This endpoint is long overdue. Just a few nits and some clarification.

api/api.go Outdated
@@ -180,14 +184,40 @@ func (api *API) status(w http.ResponseWriter, req *http.Request) {
"buildDate": version.BuildDate,
"goVersion": version.GoVersion,
},
Uptime: api.uptime,
Uptime: api.uptime,
MeshStatus: getStrippedDownMeshStatus(api),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove the getStrippedDown ... meshStatus is perfect :)

api/api.go Outdated
}

api.mtx.RUnlock()

respond(w, status)
}

type strippedDownMeshStatus struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here: strippedDown

api/api.go Outdated
Connections []strippedDownLocalConnectionStatus `json:"connections"`
}

type strippedDownLocalConnectionStatus struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here: strippedDown

api/api.go Outdated

type strippedDownLocalConnectionStatus struct {
Address string `json:"address"`
State string `json:"state"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe describe in a comment what Address and State will look like. While I can imagine that Address will be an IPv4 address, an IPv6 address or a hostname I have no idea what State could be (maybe "healthy"?).

api/api.go Outdated

func getStrippedDownMeshStatus(api *API) strippedDownMeshStatus {
status := mesh.NewStatus(api.mrouter)
strippedStatus := strippedDownMeshStatus{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically same here stripped, but I can see that that will cause collisions, generally speaking though you can keep the variable names much shorter. For example:

s := mesh.NewStatus(api.mrouter)
status := meshStatus{
...
}

@fabxc
Copy link
Contributor

fabxc commented Mar 7, 2017

Are we sure we want to use the information from Status.Connections? It seems from the user perspective we'd mostly care about connected peers, i.e. other Alertmanager instances.
There's a separate slice for that in the status object.

https://godoc.org/github.com/weaveworks/mesh#PeerStatus

Connections in the mesh will grow in complexity as we have more connected peers. The number of peers will remain overviewable.

@stuartnelson3
Copy link
Contributor

Exposing that information as some form of data visualization might be really cool (and infinitely more grokkable).

I agree with @fabxc, as a user I would rather see which peers are in "my" mesh than know which ones a particular node is actively connected to.

@brancz
Copy link
Member

brancz commented Mar 7, 2017

I was thinking those are probably the same, but looked a bit more into it and agree that a user is probably more interested in the peers themselves then the connections.

Now meshStatus contains all peers (Name, NickName, UID) of the network.
Additionally adding Name and Nickname of current target to toplevel of
meshNetwork object.
@fabxc
Copy link
Contributor

fabxc commented Mar 7, 2017

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants