From f471f91890ff426ec477014d34bd7d157be81349 Mon Sep 17 00:00:00 2001 From: dmathieu Date: Fri, 1 Sep 2023 11:02:57 +0200 Subject: [PATCH 1/5] add otel metrics to HTTP middleware --- internal/beater/api/mux.go | 2 +- .../middleware/monitoring_middleware.go | 71 +++++++++++++++---- .../middleware/monitoring_middleware_test.go | 65 ++++++++++++++--- .../beater/monitoringtest/opentelemetry.go | 55 ++++++++++++++ 4 files changed, 170 insertions(+), 23 deletions(-) create mode 100644 internal/beater/monitoringtest/opentelemetry.go diff --git a/internal/beater/api/mux.go b/internal/beater/api/mux.go index d86e407b1a2..a42b22f4a09 100644 --- a/internal/beater/api/mux.go +++ b/internal/beater/api/mux.go @@ -260,7 +260,7 @@ func apmMiddleware(m map[request.ResultID]*monitoring.Int) []middleware.Middlewa middleware.LogMiddleware(), middleware.TimeoutMiddleware(), middleware.RecoverPanicMiddleware(), - middleware.MonitoringMiddleware(m), + middleware.MonitoringMiddleware(m, nil), } } diff --git a/internal/beater/middleware/monitoring_middleware.go b/internal/beater/middleware/monitoring_middleware.go index 2b789c2be4a..80bb066e051 100644 --- a/internal/beater/middleware/monitoring_middleware.go +++ b/internal/beater/middleware/monitoring_middleware.go @@ -18,36 +18,79 @@ package middleware import ( + "context" "net/http" - "github.com/elastic/elastic-agent-libs/monitoring" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/metric" "github.com/elastic/apm-server/internal/beater/request" + "github.com/elastic/elastic-agent-libs/monitoring" ) -// MonitoringMiddleware returns a middleware that increases monitoring counters for collecting metrics -// about request processing. As input parameter it takes a map capable of mapping a request.ResultID to a counter. -func MonitoringMiddleware(m map[request.ResultID]*monitoring.Int) Middleware { +type monitoringMiddleware struct { + meter metric.Meter + + ints map[request.ResultID]*monitoring.Int + counters map[request.ResultID]metric.Int64Counter +} + +func (m *monitoringMiddleware) Middleware() Middleware { return func(h request.Handler) (request.Handler, error) { - inc := func(id request.ResultID) { - if counter, ok := m[id]; ok { - counter.Inc() - } - } return func(c *request.Context) { - inc(request.IDRequestCount) + ctx := context.Background() + + m.getMetric(request.IDRequestCount).Add(ctx, 1) + m.inc(request.IDRequestCount) h(c) - inc(request.IDResponseCount) + m.getMetric(request.IDResponseCount).Add(ctx, 1) + m.inc(request.IDResponseCount) if c.Result.StatusCode >= http.StatusBadRequest { - inc(request.IDResponseErrorsCount) + m.getMetric(request.IDResponseErrorsCount).Add(ctx, 1) + m.inc(request.IDResponseErrorsCount) } else { - inc(request.IDResponseValidCount) + m.getMetric(request.IDResponseValidCount).Add(ctx, 1) + m.inc(request.IDResponseValidCount) } - inc(c.Result.ID) + m.getMetric(c.Result.ID).Add(ctx, 1) + m.inc(c.Result.ID) }, nil } } + +func (m *monitoringMiddleware) inc(id request.ResultID) { + if counter, ok := m.ints[id]; ok { + counter.Inc() + } +} + +func (m *monitoringMiddleware) getMetric(n request.ResultID) metric.Int64Counter { + name := "http." + n + if met, ok := m.counters[name]; ok { + return met + } + + nm, _ := m.meter.Int64Counter(string(name)) + m.counters[name] = nm + return nm +} + +// MonitoringMiddleware returns a middleware that increases monitoring counters for collecting metrics +// about request processing. As input parameter it takes a map capable of mapping a request.ResultID to a counter. +func MonitoringMiddleware(m map[request.ResultID]*monitoring.Int, mp metric.MeterProvider) Middleware { + if mp == nil { + mp = otel.GetMeterProvider() + } + + mid := &monitoringMiddleware{ + meter: mp.Meter("internal/beater/middleware"), + ints: m, + counters: map[request.ResultID]metric.Int64Counter{}, + } + + return mid.Middleware() +} diff --git a/internal/beater/middleware/monitoring_middleware_test.go b/internal/beater/middleware/monitoring_middleware_test.go index b4ad6c18ff8..cd1a7fb104e 100644 --- a/internal/beater/middleware/monitoring_middleware_test.go +++ b/internal/beater/middleware/monitoring_middleware_test.go @@ -21,6 +21,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" "github.com/elastic/elastic-agent-libs/monitoring" @@ -38,13 +40,23 @@ func TestMonitoringHandler(t *testing.T) { checkMonitoring := func(t *testing.T, h func(*request.Context), expected map[request.ResultID]int, + expectedOtel map[string]int64, m map[request.ResultID]*monitoring.Int, ) { + reader := sdkmetric.NewManualReader(sdkmetric.WithTemporalitySelector( + func(ik sdkmetric.InstrumentKind) metricdata.Temporality { + return metricdata.DeltaTemporality + }, + )) + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + monitoringtest.ClearRegistry(m) c, _ := DefaultContextWithResponseRecorder() - Apply(MonitoringMiddleware(m), h)(c) + Apply(MonitoringMiddleware(m, mp), h)(c) equal, result := monitoringtest.CompareMonitoringInt(expected, m) assert.True(t, equal, result) + + monitoringtest.ExpectOtelMetrics(t, reader, expectedOtel) } t.Run("Error", func(t *testing.T) { @@ -54,8 +66,16 @@ func TestMonitoringHandler(t *testing.T) { request.IDRequestCount: 1, request.IDResponseCount: 1, request.IDResponseErrorsCount: 1, - request.IDResponseErrorsForbidden: 1}, - mockMonitoring) + request.IDResponseErrorsForbidden: 1, + }, + map[string]int64{ + "http." + string(request.IDRequestCount): 1, + "http." + string(request.IDResponseCount): 1, + "http." + string(request.IDResponseErrorsCount): 1, + "http." + string(request.IDResponseErrorsForbidden): 1, + }, + mockMonitoring, + ) }) t.Run("Accepted", func(t *testing.T) { @@ -65,8 +85,16 @@ func TestMonitoringHandler(t *testing.T) { request.IDRequestCount: 1, request.IDResponseCount: 1, request.IDResponseValidCount: 1, - request.IDResponseValidAccepted: 1}, - mockMonitoring) + request.IDResponseValidAccepted: 1, + }, + map[string]int64{ + "http." + string(request.IDRequestCount): 1, + "http." + string(request.IDResponseCount): 1, + "http." + string(request.IDResponseValidCount): 1, + "http." + string(request.IDResponseValidAccepted): 1, + }, + mockMonitoring, + ) }) t.Run("Idle", func(t *testing.T) { @@ -76,8 +104,16 @@ func TestMonitoringHandler(t *testing.T) { request.IDRequestCount: 1, request.IDResponseCount: 1, request.IDResponseValidCount: 1, - request.IDUnset: 1}, - mockMonitoring) + request.IDUnset: 1, + }, + map[string]int64{ + "http." + string(request.IDRequestCount): 1, + "http." + string(request.IDResponseCount): 1, + "http." + string(request.IDResponseValidCount): 1, + "http." + string(request.IDUnset): 1, + }, + mockMonitoring, + ) }) t.Run("Panic", func(t *testing.T) { @@ -89,6 +125,12 @@ func TestMonitoringHandler(t *testing.T) { request.IDResponseErrorsCount: 1, request.IDResponseErrorsInternal: 1, }, + map[string]int64{ + "http." + string(request.IDRequestCount): 1, + "http." + string(request.IDResponseCount): 1, + "http." + string(request.IDResponseErrorsCount): 1, + "http." + string(request.IDResponseErrorsInternal): 1, + }, mockMonitoring) }) @@ -96,6 +138,13 @@ func TestMonitoringHandler(t *testing.T) { checkMonitoring(t, HandlerIdle, map[request.ResultID]int{}, - mockMonitoringNil) + map[string]int64{ + "http." + string(request.IDRequestCount): 1, + "http." + string(request.IDResponseCount): 1, + "http." + string(request.IDResponseValidCount): 1, + "http." + string(request.IDUnset): 1, + }, + mockMonitoringNil, + ) }) } diff --git a/internal/beater/monitoringtest/opentelemetry.go b/internal/beater/monitoringtest/opentelemetry.go new file mode 100644 index 00000000000..3263014d4a8 --- /dev/null +++ b/internal/beater/monitoringtest/opentelemetry.go @@ -0,0 +1,55 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package monitoringtest + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" +) + +func ExpectOtelMetrics(t *testing.T, reader sdkmetric.Reader, expectedMetrics map[string]int64) { + t.Helper() + + var rm metricdata.ResourceMetrics + assert.NoError(t, reader.Collect(context.Background(), &rm)) + + assert.NotEqual(t, 0, len(rm.ScopeMetrics)) + foundMetrics := []string{} + for _, sm := range rm.ScopeMetrics { + + for _, m := range sm.Metrics { + switch d := m.Data.(type) { + case metricdata.Sum[int64]: + assert.Equal(t, 1, len(d.DataPoints)) + foundMetrics = append(foundMetrics, m.Name) + + if v, ok := expectedMetrics[m.Name]; ok { + assert.Equal(t, v, d.DataPoints[0].Value, m.Name) + } else { + assert.Fail(t, "unexpected metric", m.Name) + } + } + } + } + + assert.Equal(t, len(expectedMetrics), len(foundMetrics)) +} From 7ba7d48af9734f93ceea5d5552c1c398e9404134 Mon Sep 17 00:00:00 2001 From: dmathieu Date: Fri, 1 Sep 2023 11:22:38 +0200 Subject: [PATCH 2/5] add otel metrics to gRPC interceptor --- internal/beater/beater.go | 2 +- internal/beater/interceptors/metrics.go | 86 ++++++++++++++----- internal/beater/interceptors/metrics_test.go | 62 ++++++++++++- .../beater/monitoringtest/opentelemetry.go | 6 +- internal/beater/otlp/grpc_test.go | 2 +- 5 files changed, 129 insertions(+), 29 deletions(-) diff --git a/internal/beater/beater.go b/internal/beater/beater.go index a07e248385d..8b61c044790 100644 --- a/internal/beater/beater.go +++ b/internal/beater/beater.go @@ -387,7 +387,7 @@ func (s *Runner) Run(ctx context.Context) error { apmgrpc.NewUnaryServerInterceptor(apmgrpc.WithRecovery(), apmgrpc.WithTracer(tracer)), interceptors.ClientMetadata(), interceptors.Logging(gRPCLogger), - interceptors.Metrics(gRPCLogger), + interceptors.Metrics(gRPCLogger, nil), interceptors.Timeout(), interceptors.Auth(authenticator), interceptors.AnonymousRateLimit(ratelimitStore), diff --git a/internal/beater/interceptors/metrics.go b/internal/beater/interceptors/metrics.go index a5c130f681c..a1e2bbb0633 100644 --- a/internal/beater/interceptors/metrics.go +++ b/internal/beater/interceptors/metrics.go @@ -20,6 +20,8 @@ package interceptors import ( "context" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/metric" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -47,38 +49,38 @@ type UnaryRequestMetrics interface { RequestMetrics(fullMethod string) map[request.ResultID]*monitoring.Int } -// Metrics returns a grpc.UnaryServerInterceptor that increments metrics -// for gRPC method calls. -// -// If a gRPC service implements UnaryRequestMetrics, its RequestMetrics -// method will be called to obtain the metrics map for incrementing. If the -// service does not implement UnaryRequestMetrics, but -// RegisterMethodUnaryRequestMetrics has been called for the invoked method, -// then the registered UnaryRequestMetrics will be used instead. Finally, -// if neither of these are available, a warning will be logged and no metrics -// will be gathered. -func Metrics(logger *logp.Logger) grpc.UnaryServerInterceptor { +type metricsInterceptor struct { + logger *logp.Logger + meter metric.Meter + + counters map[request.ResultID]metric.Int64Counter +} + +func (m *metricsInterceptor) Interceptor() grpc.UnaryServerInterceptor { return func( ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler, ) (interface{}, error) { - var m map[request.ResultID]*monitoring.Int + var ints map[request.ResultID]*monitoring.Int if requestMetrics, ok := info.Server.(UnaryRequestMetrics); ok { - m = requestMetrics.RequestMetrics(info.FullMethod) + ints = requestMetrics.RequestMetrics(info.FullMethod) } else { - m = methodUnaryRequestMetrics[info.FullMethod] + ints = methodUnaryRequestMetrics[info.FullMethod] } - if m == nil { - logger.With( + if ints == nil { + m.logger.With( "grpc.request.method", info.FullMethod, ).Warn("metrics registry missing") return handler(ctx, req) } - m[request.IDRequestCount].Inc() - defer m[request.IDResponseCount].Inc() + m.getMetric(request.IDRequestCount).Add(ctx, 1) + defer m.getMetric(request.IDResponseCount).Add(ctx, 1) + + ints[request.IDRequestCount].Inc() + defer ints[request.IDResponseCount].Inc() resp, err := handler(ctx, req) @@ -88,17 +90,57 @@ func Metrics(logger *logp.Logger) grpc.UnaryServerInterceptor { if s, ok := status.FromError(err); ok { switch s.Code() { case codes.Unauthenticated: - m[request.IDResponseErrorsUnauthorized].Inc() + m.getMetric(request.IDResponseErrorsUnauthorized).Add(ctx, 1) + ints[request.IDResponseErrorsUnauthorized].Inc() case codes.DeadlineExceeded, codes.Canceled: - m[request.IDResponseErrorsTimeout].Inc() + m.getMetric(request.IDResponseErrorsTimeout).Add(ctx, 1) + ints[request.IDResponseErrorsTimeout].Inc() case codes.ResourceExhausted: - m[request.IDResponseErrorsRateLimit].Inc() + m.getMetric(request.IDResponseErrorsRateLimit).Add(ctx, 1) + ints[request.IDResponseErrorsRateLimit].Inc() } } } - m[responseID].Inc() + m.getMetric(responseID).Add(ctx, 1) + ints[responseID].Inc() return resp, err } } + +func (m *metricsInterceptor) getMetric(n request.ResultID) metric.Int64Counter { + name := "grpc." + n + if met, ok := m.counters[name]; ok { + return met + } + + nm, _ := m.meter.Int64Counter(string(name)) + m.counters[name] = nm + return nm +} + +// Metrics returns a grpc.UnaryServerInterceptor that increments metrics +// for gRPC method calls. +// +// If a gRPC service implements UnaryRequestMetrics, its RequestMetrics +// method will be called to obtain the metrics map for incrementing. If the +// service does not implement UnaryRequestMetrics, but +// RegisterMethodUnaryRequestMetrics has been called for the invoked method, +// then the registered UnaryRequestMetrics will be used instead. Finally, +// if neither of these are available, a warning will be logged and no metrics +// will be gathered. +func Metrics(logger *logp.Logger, mp metric.MeterProvider) grpc.UnaryServerInterceptor { + if mp == nil { + mp = otel.GetMeterProvider() + } + + i := &metricsInterceptor{ + logger: logger, + meter: mp.Meter("internal/beater/interceptors"), + + counters: map[request.ResultID]metric.Int64Counter{}, + } + + return i.Interceptor() +} diff --git a/internal/beater/interceptors/metrics_test.go b/internal/beater/interceptors/metrics_test.go index c675cb4559c..5fd5e119333 100644 --- a/internal/beater/interceptors/metrics_test.go +++ b/internal/beater/interceptors/metrics_test.go @@ -23,6 +23,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -41,13 +43,20 @@ var monitoringKeys = append( ) func TestMetrics(t *testing.T) { + reader := sdkmetric.NewManualReader(sdkmetric.WithTemporalitySelector( + func(ik sdkmetric.InstrumentKind) metricdata.Temporality { + return metricdata.DeltaTemporality + }, + )) + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + registry := monitoring.NewRegistry() monitoringMap := request.MonitoringMapForRegistry(registry, monitoringKeys) methodName := "test_method_name" logger := logp.NewLogger("interceptor.metrics.test") - interceptor := Metrics(logger) + interceptor := Metrics(logger, mp) ctx := context.Background() info := &grpc.UnaryServerInfo{ @@ -59,10 +68,13 @@ func TestMetrics(t *testing.T) { } for _, tc := range []struct { + name string f func(ctx context.Context, req interface{}) (interface{}, error) monitoringInt map[request.ResultID]int64 + expectedOtel map[string]int64 }{ { + name: "with an error", f: func(ctx context.Context, req interface{}) (interface{}, error) { return nil, errors.New("error") }, @@ -76,8 +88,14 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsTimeout: 0, request.IDResponseErrorsUnauthorized: 0, }, + expectedOtel: map[string]int64{ + "grpc." + string(request.IDRequestCount): 1, + "grpc." + string(request.IDResponseCount): 1, + "grpc." + string(request.IDResponseErrorsCount): 1, + }, }, { + name: "with an unauthenticated error", f: func(ctx context.Context, req interface{}) (interface{}, error) { return nil, status.Error(codes.Unauthenticated, "error") }, @@ -91,8 +109,15 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsTimeout: 0, request.IDResponseErrorsUnauthorized: 1, }, + expectedOtel: map[string]int64{ + "grpc." + string(request.IDRequestCount): 1, + "grpc." + string(request.IDResponseCount): 1, + "grpc." + string(request.IDResponseErrorsCount): 1, + "grpc." + string(request.IDResponseErrorsUnauthorized): 1, + }, }, { + name: "with a deadline exceeded error", f: func(ctx context.Context, req interface{}) (interface{}, error) { return nil, status.Error(codes.DeadlineExceeded, "request timed out") }, @@ -106,8 +131,15 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsTimeout: 1, request.IDResponseErrorsUnauthorized: 0, }, + expectedOtel: map[string]int64{ + "grpc." + string(request.IDRequestCount): 1, + "grpc." + string(request.IDResponseCount): 1, + "grpc." + string(request.IDResponseErrorsCount): 1, + "grpc." + string(request.IDResponseErrorsTimeout): 1, + }, }, { + name: "with a canceled error", f: func(ctx context.Context, req interface{}) (interface{}, error) { return nil, status.Error(codes.Canceled, "request timed out") }, @@ -121,8 +153,15 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsTimeout: 1, request.IDResponseErrorsUnauthorized: 0, }, + expectedOtel: map[string]int64{ + "grpc." + string(request.IDRequestCount): 1, + "grpc." + string(request.IDResponseCount): 1, + "grpc." + string(request.IDResponseErrorsCount): 1, + "grpc." + string(request.IDResponseErrorsTimeout): 1, + }, }, { + name: "with a resource exhausted error", f: func(ctx context.Context, req interface{}) (interface{}, error) { return nil, status.Error(codes.ResourceExhausted, "rate limit exceeded") }, @@ -136,8 +175,15 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsTimeout: 0, request.IDResponseErrorsUnauthorized: 0, }, + expectedOtel: map[string]int64{ + "grpc." + string(request.IDRequestCount): 1, + "grpc." + string(request.IDResponseCount): 1, + "grpc." + string(request.IDResponseErrorsCount): 1, + "grpc." + string(request.IDResponseErrorsRateLimit): 1, + }, }, { + name: "with a success", f: func(ctx context.Context, req interface{}) (interface{}, error) { return nil, nil }, @@ -151,11 +197,19 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsTimeout: 0, request.IDResponseErrorsUnauthorized: 0, }, + expectedOtel: map[string]int64{ + "grpc." + string(request.IDRequestCount): 1, + "grpc." + string(request.IDResponseCount): 1, + "grpc." + string(request.IDResponseValidCount): 1, + }, }, } { - interceptor(ctx, nil, info, tc.f) - assertMonitoring(t, tc.monitoringInt, monitoringMap) - monitoringtest.ClearRegistry(monitoringMap) + t.Run(tc.name, func(t *testing.T) { + interceptor(ctx, nil, info, tc.f) + assertMonitoring(t, tc.monitoringInt, monitoringMap) + monitoringtest.ClearRegistry(monitoringMap) + monitoringtest.ExpectOtelMetrics(t, reader, tc.expectedOtel) + }) } } diff --git a/internal/beater/monitoringtest/opentelemetry.go b/internal/beater/monitoringtest/opentelemetry.go index 3263014d4a8..8ed3acc9f4a 100644 --- a/internal/beater/monitoringtest/opentelemetry.go +++ b/internal/beater/monitoringtest/opentelemetry.go @@ -51,5 +51,9 @@ func ExpectOtelMetrics(t *testing.T, reader sdkmetric.Reader, expectedMetrics ma } } - assert.Equal(t, len(expectedMetrics), len(foundMetrics)) + expectedMetricsKeys := []string{} + for k, _ := range expectedMetrics { + expectedMetricsKeys = append(expectedMetricsKeys, k) + } + assert.ElementsMatch(t, expectedMetricsKeys, foundMetrics) } diff --git a/internal/beater/otlp/grpc_test.go b/internal/beater/otlp/grpc_test.go index 25c8c4207e2..7f3bfb7877b 100644 --- a/internal/beater/otlp/grpc_test.go +++ b/internal/beater/otlp/grpc_test.go @@ -190,7 +190,7 @@ func newGRPCServer(t *testing.T, batchProcessor modelpb.BatchProcessor) *grpc.Cl lis, err := net.Listen("tcp", "localhost:0") require.NoError(t, err) logger := logp.NewLogger("otlp.grpc.test") - srv := grpc.NewServer(grpc.UnaryInterceptor(interceptors.Metrics(logger))) + srv := grpc.NewServer(grpc.UnaryInterceptor(interceptors.Metrics(logger, nil))) semaphore := semaphore.NewWeighted(1) otlp.RegisterGRPCServices(srv, zap.NewNop(), batchProcessor, semaphore) From c20602b5cebb4cac9b97547ed86f8d6a43576b71 Mon Sep 17 00:00:00 2001 From: dmathieu Date: Fri, 1 Sep 2023 11:26:19 +0200 Subject: [PATCH 3/5] add server to metrics names --- internal/beater/interceptors/metrics.go | 2 +- internal/beater/interceptors/metrics_test.go | 44 +++++++++---------- .../middleware/monitoring_middleware.go | 2 +- .../middleware/monitoring_middleware_test.go | 40 ++++++++--------- 4 files changed, 44 insertions(+), 44 deletions(-) diff --git a/internal/beater/interceptors/metrics.go b/internal/beater/interceptors/metrics.go index a1e2bbb0633..9d4012b9b77 100644 --- a/internal/beater/interceptors/metrics.go +++ b/internal/beater/interceptors/metrics.go @@ -110,7 +110,7 @@ func (m *metricsInterceptor) Interceptor() grpc.UnaryServerInterceptor { } func (m *metricsInterceptor) getMetric(n request.ResultID) metric.Int64Counter { - name := "grpc." + n + name := "grpc.server." + n if met, ok := m.counters[name]; ok { return met } diff --git a/internal/beater/interceptors/metrics_test.go b/internal/beater/interceptors/metrics_test.go index 5fd5e119333..892a7b9e6d4 100644 --- a/internal/beater/interceptors/metrics_test.go +++ b/internal/beater/interceptors/metrics_test.go @@ -89,9 +89,9 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsUnauthorized: 0, }, expectedOtel: map[string]int64{ - "grpc." + string(request.IDRequestCount): 1, - "grpc." + string(request.IDResponseCount): 1, - "grpc." + string(request.IDResponseErrorsCount): 1, + "grpc.server." + string(request.IDRequestCount): 1, + "grpc.server." + string(request.IDResponseCount): 1, + "grpc.server." + string(request.IDResponseErrorsCount): 1, }, }, { @@ -110,10 +110,10 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsUnauthorized: 1, }, expectedOtel: map[string]int64{ - "grpc." + string(request.IDRequestCount): 1, - "grpc." + string(request.IDResponseCount): 1, - "grpc." + string(request.IDResponseErrorsCount): 1, - "grpc." + string(request.IDResponseErrorsUnauthorized): 1, + "grpc.server." + string(request.IDRequestCount): 1, + "grpc.server." + string(request.IDResponseCount): 1, + "grpc.server." + string(request.IDResponseErrorsCount): 1, + "grpc.server." + string(request.IDResponseErrorsUnauthorized): 1, }, }, { @@ -132,10 +132,10 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsUnauthorized: 0, }, expectedOtel: map[string]int64{ - "grpc." + string(request.IDRequestCount): 1, - "grpc." + string(request.IDResponseCount): 1, - "grpc." + string(request.IDResponseErrorsCount): 1, - "grpc." + string(request.IDResponseErrorsTimeout): 1, + "grpc.server." + string(request.IDRequestCount): 1, + "grpc.server." + string(request.IDResponseCount): 1, + "grpc.server." + string(request.IDResponseErrorsCount): 1, + "grpc.server." + string(request.IDResponseErrorsTimeout): 1, }, }, { @@ -154,10 +154,10 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsUnauthorized: 0, }, expectedOtel: map[string]int64{ - "grpc." + string(request.IDRequestCount): 1, - "grpc." + string(request.IDResponseCount): 1, - "grpc." + string(request.IDResponseErrorsCount): 1, - "grpc." + string(request.IDResponseErrorsTimeout): 1, + "grpc.server." + string(request.IDRequestCount): 1, + "grpc.server." + string(request.IDResponseCount): 1, + "grpc.server." + string(request.IDResponseErrorsCount): 1, + "grpc.server." + string(request.IDResponseErrorsTimeout): 1, }, }, { @@ -176,10 +176,10 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsUnauthorized: 0, }, expectedOtel: map[string]int64{ - "grpc." + string(request.IDRequestCount): 1, - "grpc." + string(request.IDResponseCount): 1, - "grpc." + string(request.IDResponseErrorsCount): 1, - "grpc." + string(request.IDResponseErrorsRateLimit): 1, + "grpc.server." + string(request.IDRequestCount): 1, + "grpc.server." + string(request.IDResponseCount): 1, + "grpc.server." + string(request.IDResponseErrorsCount): 1, + "grpc.server." + string(request.IDResponseErrorsRateLimit): 1, }, }, { @@ -198,9 +198,9 @@ func TestMetrics(t *testing.T) { request.IDResponseErrorsUnauthorized: 0, }, expectedOtel: map[string]int64{ - "grpc." + string(request.IDRequestCount): 1, - "grpc." + string(request.IDResponseCount): 1, - "grpc." + string(request.IDResponseValidCount): 1, + "grpc.server." + string(request.IDRequestCount): 1, + "grpc.server." + string(request.IDResponseCount): 1, + "grpc.server." + string(request.IDResponseValidCount): 1, }, }, } { diff --git a/internal/beater/middleware/monitoring_middleware.go b/internal/beater/middleware/monitoring_middleware.go index 80bb066e051..1135c955676 100644 --- a/internal/beater/middleware/monitoring_middleware.go +++ b/internal/beater/middleware/monitoring_middleware.go @@ -69,7 +69,7 @@ func (m *monitoringMiddleware) inc(id request.ResultID) { } func (m *monitoringMiddleware) getMetric(n request.ResultID) metric.Int64Counter { - name := "http." + n + name := "http.server." + n if met, ok := m.counters[name]; ok { return met } diff --git a/internal/beater/middleware/monitoring_middleware_test.go b/internal/beater/middleware/monitoring_middleware_test.go index cd1a7fb104e..04f2cd3f141 100644 --- a/internal/beater/middleware/monitoring_middleware_test.go +++ b/internal/beater/middleware/monitoring_middleware_test.go @@ -69,10 +69,10 @@ func TestMonitoringHandler(t *testing.T) { request.IDResponseErrorsForbidden: 1, }, map[string]int64{ - "http." + string(request.IDRequestCount): 1, - "http." + string(request.IDResponseCount): 1, - "http." + string(request.IDResponseErrorsCount): 1, - "http." + string(request.IDResponseErrorsForbidden): 1, + "http.server." + string(request.IDRequestCount): 1, + "http.server." + string(request.IDResponseCount): 1, + "http.server." + string(request.IDResponseErrorsCount): 1, + "http.server." + string(request.IDResponseErrorsForbidden): 1, }, mockMonitoring, ) @@ -88,10 +88,10 @@ func TestMonitoringHandler(t *testing.T) { request.IDResponseValidAccepted: 1, }, map[string]int64{ - "http." + string(request.IDRequestCount): 1, - "http." + string(request.IDResponseCount): 1, - "http." + string(request.IDResponseValidCount): 1, - "http." + string(request.IDResponseValidAccepted): 1, + "http.server." + string(request.IDRequestCount): 1, + "http.server." + string(request.IDResponseCount): 1, + "http.server." + string(request.IDResponseValidCount): 1, + "http.server." + string(request.IDResponseValidAccepted): 1, }, mockMonitoring, ) @@ -107,10 +107,10 @@ func TestMonitoringHandler(t *testing.T) { request.IDUnset: 1, }, map[string]int64{ - "http." + string(request.IDRequestCount): 1, - "http." + string(request.IDResponseCount): 1, - "http." + string(request.IDResponseValidCount): 1, - "http." + string(request.IDUnset): 1, + "http.server." + string(request.IDRequestCount): 1, + "http.server." + string(request.IDResponseCount): 1, + "http.server." + string(request.IDResponseValidCount): 1, + "http.server." + string(request.IDUnset): 1, }, mockMonitoring, ) @@ -126,10 +126,10 @@ func TestMonitoringHandler(t *testing.T) { request.IDResponseErrorsInternal: 1, }, map[string]int64{ - "http." + string(request.IDRequestCount): 1, - "http." + string(request.IDResponseCount): 1, - "http." + string(request.IDResponseErrorsCount): 1, - "http." + string(request.IDResponseErrorsInternal): 1, + "http.server." + string(request.IDRequestCount): 1, + "http.server." + string(request.IDResponseCount): 1, + "http.server." + string(request.IDResponseErrorsCount): 1, + "http.server." + string(request.IDResponseErrorsInternal): 1, }, mockMonitoring) }) @@ -139,10 +139,10 @@ func TestMonitoringHandler(t *testing.T) { HandlerIdle, map[request.ResultID]int{}, map[string]int64{ - "http." + string(request.IDRequestCount): 1, - "http." + string(request.IDResponseCount): 1, - "http." + string(request.IDResponseValidCount): 1, - "http." + string(request.IDUnset): 1, + "http.server." + string(request.IDRequestCount): 1, + "http.server." + string(request.IDResponseCount): 1, + "http.server." + string(request.IDResponseValidCount): 1, + "http.server." + string(request.IDUnset): 1, }, mockMonitoringNil, ) From 5c20dd85e973045405474a6d7c3d6d1a2e4086b9 Mon Sep 17 00:00:00 2001 From: dmathieu Date: Fri, 1 Sep 2023 11:33:04 +0200 Subject: [PATCH 4/5] remove unnecessary assignment --- internal/beater/monitoringtest/opentelemetry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/beater/monitoringtest/opentelemetry.go b/internal/beater/monitoringtest/opentelemetry.go index 8ed3acc9f4a..e421d45d3f7 100644 --- a/internal/beater/monitoringtest/opentelemetry.go +++ b/internal/beater/monitoringtest/opentelemetry.go @@ -52,7 +52,7 @@ func ExpectOtelMetrics(t *testing.T, reader sdkmetric.Reader, expectedMetrics ma } expectedMetricsKeys := []string{} - for k, _ := range expectedMetrics { + for k := range expectedMetrics { expectedMetricsKeys = append(expectedMetricsKeys, k) } assert.ElementsMatch(t, expectedMetricsKeys, foundMetrics) From 0e36cccdd41c6e930d776c268a9fead7bbbab7ba Mon Sep 17 00:00:00 2001 From: dmathieu Date: Mon, 4 Sep 2023 11:55:37 +0200 Subject: [PATCH 5/5] remove metrics that doesn't appear to be used --- internal/beater/interceptors/metrics_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/beater/interceptors/metrics_test.go b/internal/beater/interceptors/metrics_test.go index 892a7b9e6d4..a225f1b7eb8 100644 --- a/internal/beater/interceptors/metrics_test.go +++ b/internal/beater/interceptors/metrics_test.go @@ -83,7 +83,6 @@ func TestMetrics(t *testing.T) { request.IDResponseCount: 1, request.IDResponseValidCount: 0, request.IDResponseErrorsCount: 1, - request.IDResponseErrorsInternal: 1, request.IDResponseErrorsRateLimit: 0, request.IDResponseErrorsTimeout: 0, request.IDResponseErrorsUnauthorized: 0,