feat: add prometheus metrics support#37
feat: add prometheus metrics support#37j-bro wants to merge 9 commits intorocketsciencegg:masterfrom
Conversation
cmd/cli/bulk.go
Outdated
| } | ||
|
|
||
| client, err := svrquery.NewClient(querySection, addressSection, options...) | ||
| client, err := svrquery.NewUDPClient(querySection, addressSection, options...) |
lwaddicor
left a comment
lwaddicor
left a comment
There was a problem hiding this comment.
Starting to form up, a few points of feedback
cmd/cli/main.go
Outdated
| "github.com/multiplay/go-svrquery/lib/svrquery/protocol" | ||
| "github.com/multiplay/go-svrquery/lib/svrsample" | ||
| "github.com/multiplay/go-svrquery/lib/svrsample/common" | ||
| "log" |
There was a problem hiding this comment.
Import ordering here is wrong
lib/svrquery/client.go
Outdated
| // NewClient creates a new client that talks to addr. | ||
| func NewClient(proto, addr string, options ...Option) (*Client, error) { | ||
| // NewUDPClient creates a new client that talks to addr. | ||
| func NewUDPClient(proto, addr string, options ...Option) (*UDPClient, error) { |
There was a problem hiding this comment.
I think this is going to break users of the library as NewClient is publicly exported and the main way people will interact with the client.
Can we either have the struct do both or abstract it internally?
lib/svrquery/protocol/interfaces.go
Outdated
| // Client is an interface which is implemented by types which can act a query transport. | ||
| type Client interface { | ||
| io.ReadWriteCloser | ||
| Queryer |
There was a problem hiding this comment.
Out of interest, is this just usability? I see we populate this from the protocol.Get(...) call so seems sensible but just checking
There was a problem hiding this comment.
Mainly since I now have 2 types of clients, the interface was needed so that main.go can use them as both io.ReadWriteCloser and Queryer (the UDP client implementation already implemented both). But I'll rework the client to make it backwards compatible so this might change again.
lib/svrquery/protocol/prom/query.go
Outdated
| resp.MaxPlayers = *v.Metric[0].Gauge.Value | ||
| case serverInfoMetricName: | ||
| if len(v.Metric) == 0 || v.Metric[0] == nil || len(v.Metric[0].Label) == 0 { | ||
| slog.Error("server_info metric is missing labels") |
There was a problem hiding this comment.
Logging messages like this will break the cli responses (e.g. when in json output mode) and in libraries is usually not done as you don't know the log format of the application running the code
2 participants
TODO