Skip to content

Commit

Permalink
Replace idiosyncratic metrics collector with expvar. (#89)
Browse files Browse the repository at this point in the history
This change removes the "metrics" package from the module and updates the
existing metrics to use the standard expvar package instead.  The package
creates a shared *expvar.Map at initialization time, and populates expvar
metrics for the existing server details (request counts, bytes read/written,
active servers, etc.).

This is a breaking change to the module API.

- tools: remove references to the metrics package
- tools: update Go toolchain to 1.18
  • Loading branch information
creachadair authored Oct 30, 2022
1 parent 22a56d6 commit db6007b
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 348 deletions.
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ There is also a working [example in the Go playground](https://go.dev/play/p/Gdd

* Package [jhttp](http://godoc.org/github.com/creachadair/jrpc2/jhttp) allows clients and servers to use HTTP as a transport.

* Package [metrics](http://godoc.org/github.com/creachadair/jrpc2/metrics) defines a server metrics collector.

* Package [server](http://godoc.org/github.com/creachadair/jrpc2/server) provides support for running a server to handle multiple connections, and an in-memory implementation for testing.

[spec]: http://www.jsonrpc.org/specification
Expand Down
66 changes: 20 additions & 46 deletions jrpc2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"encoding/json"
"errors"
"expvar"
"fmt"
"sort"
"sync"
Expand Down Expand Up @@ -541,27 +542,13 @@ func TestClient_badCallParams(t *testing.T) {
func TestServer_serverInfoMetrics(t *testing.T) {
defer leaktest.Check(t)()

const metricValue = "if red were green, green would be my favourite colour"
customMetric := new(expvar.String)
customMetric.Set(metricValue)

loc := server.NewLocal(handler.Map{
"Metricize": handler.New(func(ctx context.Context) (bool, error) {
m := jrpc2.ServerFromContext(ctx).Metrics()
if m == nil {
t.Error("Request context does not contain a metrics writer")
return false, nil
}
m.Count("counters-written", 1)
m.Count("counters-written", 2)

// Max value trackers are not accumulative.
m.SetMaxValue("max-metric-value", 1)
m.SetMaxValue("max-metric-value", 5)
m.SetMaxValue("max-metric-value", 3)
m.SetMaxValue("max-metric-value", -30337)

// Counters are accumulative, and negative deltas subtract.
m.Count("zero-sum", 0)
m.Count("zero-sum", 15)
m.Count("zero-sum", -16)
m.Count("zero-sum", 1)
jrpc2.ServerMetrics().Set("custom-metric", customMetric)
return true, nil
}),
}, nil)
Expand All @@ -571,36 +558,23 @@ func TestServer_serverInfoMetrics(t *testing.T) {
if _, err := c.Call(ctx, "Metricize", nil); err != nil {
t.Fatalf("Call(Metricize) failed: %v", err)
}
if got := s.ServerInfo().Counter["rpc.serversActive"]; got != 1 {
t.Errorf("Metric rpc.serversActive: got %d, want 1", got)
active, ok := jrpc2.ServerMetrics().Get("servers_active").(*expvar.Int)
if !ok {
t.Fatal("Metric servers_active not defined")
} else if active.Value() != 2 { // 1 for this test, 1 for the examples
t.Errorf("Metric servers_active: got %d, want 1", active.Value())
}
loc.Close()

info := s.ServerInfo()
tests := []struct {
input map[string]int64
name string
want int64 // use < 0 to test for existence only
}{
{info.Counter, "rpc.requests", 1},
{info.Counter, "counters-written", 3},
{info.Counter, "zero-sum", 0},
{info.Counter, "rpc.bytesRead", -1},
{info.Counter, "rpc.bytesWritten", -1},
{info.Counter, "rpc.serversActive", 0},
{info.MaxValue, "max-metric-value", 5},
{info.MaxValue, "rpc.bytesRead", -1},
{info.MaxValue, "rpc.bytesWritten", -1},
}
for _, test := range tests {
got, ok := test.input[test.name]
if !ok {
t.Errorf("Metric %q is not defined, but was expected", test.name)
continue
}
if test.want >= 0 && got != test.want {
t.Errorf("Wrong value for metric %q: got %d, want %d", test.name, got, test.want)
}
// When populated into a ServerInfo value, metrics should be converted to
// JSON values in a sensible way.
metric := s.ServerInfo().Metrics["custom-metric"]
if metric == nil {
t.Fatal("Custom metric not found")
} else if got, ok := metric.(json.RawMessage); !ok {
t.Fatalf("Wrong metric type: got %T, want json.RawMessage", metric)
} else if s := string(got); s != `"`+metricValue+`"` {
t.Fatalf("Wrong metric value: got %q, want %q", s, metricValue)
}
}

Expand Down
138 changes: 0 additions & 138 deletions metrics/metrics.go

This file was deleted.

108 changes: 0 additions & 108 deletions metrics/metrics_test.go

This file was deleted.

13 changes: 0 additions & 13 deletions opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"time"

"github.com/creachadair/jrpc2/code"
"github.com/creachadair/jrpc2/metrics"
)

// ServerOptions control the behaviour of a server created by NewServer.
Expand Down Expand Up @@ -46,11 +45,6 @@ type ServerOptions struct {
// If unset, the server uses a background context.
NewContext func() context.Context

// If set, use this value to record server metrics. All servers created
// from the same options will share the same metrics collector. If none is
// set, an empty collector will be created for each new server.
Metrics *metrics.M

// If nonzero this value as the server start time; otherwise, use the
// current time when Start is called. All servers created from the same
// options will share the same start time if one is set.
Expand Down Expand Up @@ -88,13 +82,6 @@ func (o *ServerOptions) newContext() func() context.Context {
return o.NewContext
}

func (s *ServerOptions) metrics() *metrics.M {
if s == nil || s.Metrics == nil {
return metrics.New()
}
return s.Metrics
}

func (s *ServerOptions) rpcLog() RPCLogger {
if s == nil || s.RPCLog == nil {
return nullRPCLogger{}
Expand Down
Loading

0 comments on commit db6007b

Please sign in to comment.