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

Replace idiosyncratic metrics collector with expvar. #89

Merged
merged 2 commits into from
Oct 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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