From b7671fe8447ab2586368764ab3af6fc246717b92 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Sun, 24 Dec 2023 09:53:25 -0500 Subject: [PATCH] internal/{observe,worker}: remove exp/event metrics Switch to an experimental metrics package of my own. Change-Id: I88cad487691787be5485b5773637e084317015ea Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/552655 TryBot-Result: Gopher Robot Reviewed-by: Tatiana Bradley LUCI-TryBot-Result: Go LUCI Run-TryBot: Jonathan Amsterdam --- go.mod | 4 +++- go.sum | 4 ++++ internal/observe/observe.go | 19 ++++++++++--------- internal/worker/server.go | 9 ++++++--- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 2b9ca498..67c67479 100644 --- a/go.mod +++ b/go.mod @@ -17,12 +17,13 @@ require ( github.com/google/go-github/v41 v41.0.0 github.com/google/osv-scanner v1.3.5 github.com/google/safehtml v0.1.0 + github.com/jba/metrics v0.1.1 + github.com/jba/metrics/otel v0.1.1 github.com/jba/templatecheck v0.7.0 github.com/lib/pq v1.10.9 github.com/shurcooL/githubv4 v0.0.0-20231126234147-1cffa1f02456 go.opentelemetry.io/otel v1.21.0 go.opentelemetry.io/otel/sdk v1.21.0 - go.opentelemetry.io/otel/sdk/metric v1.21.0 golang.org/x/exp v0.0.0-20231219180239-dc181d75b848 golang.org/x/exp/event v0.0.0-20231219180239-dc181d75b848 golang.org/x/mod v0.14.0 @@ -75,6 +76,7 @@ require ( go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.46.1 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect go.opentelemetry.io/otel/metric v1.21.0 // indirect + go.opentelemetry.io/otel/sdk/metric v1.21.0 // indirect go.opentelemetry.io/otel/trace v1.21.0 // indirect golang.org/x/crypto v0.16.0 // indirect golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a // indirect diff --git a/go.sum b/go.sum index 8a44d0fa..bb343211 100644 --- a/go.sum +++ b/go.sum @@ -136,6 +136,10 @@ github.com/googleapis/gax-go/v2 v2.12.0 h1:A+gCJKdRfqXkr+BIRGtZLibNXf0m1f9E4HG56 github.com/googleapis/gax-go/v2 v2.12.0/go.mod h1:y+aIqrI5eb1YGMVJfuV3185Ts/D7qKpsEkdD5+I6QGU= github.com/imdario/mergo v0.3.15 h1:M8XP7IuFNsqUx6VPK2P9OSmsYsI/YFaGil0uD21V3dM= github.com/imdario/mergo v0.3.15/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= +github.com/jba/metrics v0.1.1 h1:hkqa7RZwPxi7No2IOxPZSeZKW0tCVyAQ5V9L4DosJFk= +github.com/jba/metrics v0.1.1/go.mod h1:8YE9o3xYiRMg6G4WFrSCV30NhTQFcGgodbVgTJ5waqs= +github.com/jba/metrics/otel v0.1.1 h1:nJNBGBiv+WPx2AiqYXP9Iqpnk1Lcq6rOSBAgPXAvNwA= +github.com/jba/metrics/otel v0.1.1/go.mod h1:ZlrAUyEXGcvJ5US085SnVuTPXlX+Oy76jNYUNTAbkKM= github.com/jba/templatecheck v0.7.0 h1:wjTb/VhGgSFeim5zjWVePBdaMo28X74bGLSABZV+zIA= github.com/jba/templatecheck v0.7.0/go.mod h1:n1Etw+Rrw1mDDD8dDRsEKTwMZsJ98EkktgNJC6wLUGo= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= diff --git a/internal/observe/observe.go b/internal/observe/observe.go index 8e6537f4..55683068 100644 --- a/internal/observe/observe.go +++ b/internal/observe/observe.go @@ -9,6 +9,7 @@ package observe import ( "context" "net/http" + "strings" "golang.org/x/exp/event" "golang.org/x/vulndb/internal/derrors" @@ -16,8 +17,9 @@ import ( mexporter "github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric" texporter "github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace" gcppropagator "github.com/GoogleCloudPlatform/opentelemetry-operations-go/propagator" + "github.com/jba/metrics/otel" "go.opentelemetry.io/otel/propagation" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/instrumentation" sdktrace "go.opentelemetry.io/otel/sdk/trace" eotel "golang.org/x/exp/event/otel" ) @@ -27,7 +29,6 @@ type Observer struct { ctx context.Context tracerProvider *sdktrace.TracerProvider traceHandler *eotel.TraceHandler - metricHandler *eotel.MetricHandler propagator propagation.TextMapPropagator // LogHandlerFunc is invoked in [Observer.Observe] to obtain an @@ -53,10 +54,12 @@ func NewObserver(ctx context.Context, projectID, serverName string) (_ *Observer if err != nil { return nil, err } - // Initialize a MeterProvider with that periodically exports to the GCP exporter. - provider := sdkmetric.NewMeterProvider( - sdkmetric.WithReader(sdkmetric.NewPeriodicReader(mex)), - ) + // Export all registered metrics except the runtime metrics. + scope := instrumentation.Scope{Name: "vulndb/worker"} + otel.Export(scope, mex, 0, func(name string) bool { + return !strings.HasPrefix(name, "runtime/") + }) + tp := sdktrace.NewTracerProvider( // Enable tracing if there is no incoming request, or if the incoming // request is sampled. @@ -66,7 +69,6 @@ func NewObserver(ctx context.Context, projectID, serverName string) (_ *Observer ctx: ctx, tracerProvider: tp, traceHandler: eotel.NewTraceHandler(tp.Tracer(serverName)), - metricHandler: eotel.NewMetricHandler(provider.Meter(serverName)), // The propagator extracts incoming trace IDs so that we can connect our trace spans // to the incoming ones constructed by Cloud Run. propagator: propagation.NewCompositeTextMapPropagator( @@ -101,6 +103,5 @@ func (h eventHandler) Event(ctx context.Context, ev *event.Event) context.Contex if h.eh != nil { ctx = h.eh.Event(ctx, ev) } - ctx = h.o.traceHandler.Event(ctx, ev) - return h.o.metricHandler.Event(ctx, ev) + return h.o.traceHandler.Event(ctx, ev) } diff --git a/internal/worker/server.go b/internal/worker/server.go index a7c03886..6aa27945 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -21,6 +21,7 @@ import ( "cloud.google.com/go/errorreporting" "github.com/google/safehtml/template" + "github.com/jba/metrics" "golang.org/x/exp/event" "golang.org/x/sync/errgroup" "golang.org/x/vulndb/internal/cvelistrepo" @@ -286,9 +287,11 @@ func (s *Server) indexPage(w http.ResponseWriter, r *http.Request) error { return renderPage(r.Context(), w, page, s.indexTemplate) } -const metricNamespace = "vulndb/worker" +type UpdateOutcome struct { + Success bool +} -var updateCounter = event.NewCounter("updates", &event.MetricOptions{Namespace: metricNamespace}) +var updateCounters = metrics.NewCounterGroup[int64, UpdateOutcome]("updates", "calls to handleUpdate") func (s *Server) handleUpdate(w http.ResponseWriter, r *http.Request) error { err := s.doUpdate(r) @@ -301,7 +304,7 @@ func (s *Server) handleUpdate(w http.ResponseWriter, r *http.Request) error { func (s *Server) doUpdate(r *http.Request) (err error) { defer func() { success := err == nil - updateCounter.Record(r.Context(), 1, event.Bool("success", success)) + updateCounters.At(UpdateOutcome{success}).Add(1) log.Debugf(r.Context(), "recorded one /update operation in counter (success=%t)", success) }()