From 0ae9660b3c778415c05067d8e11a75aed3b254cf Mon Sep 17 00:00:00 2001 From: Artem Glazychev Date: Thu, 16 Jun 2022 15:11:22 +0700 Subject: [PATCH 1/3] Fix metrics concurrency Signed-off-by: Artem Glazychev --- pkg/networkservice/common/metrics/metadata.go | 33 +++++++++++++++++++ pkg/networkservice/common/metrics/server.go | 13 ++++---- 2 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 pkg/networkservice/common/metrics/metadata.go diff --git a/pkg/networkservice/common/metrics/metadata.go b/pkg/networkservice/common/metrics/metadata.go new file mode 100644 index 000000000..788b1ab90 --- /dev/null +++ b/pkg/networkservice/common/metrics/metadata.go @@ -0,0 +1,33 @@ +// Copyright (c) 2022 Cisco and/or its affiliates. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed 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 metrics + +import ( + "context" + + "go.opentelemetry.io/otel/metric" + + "github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata" +) + +type keyType struct{} +type metricsMap map[string]metric.Int64Histogram + +func loadOrStore(ctx context.Context, metrics metricsMap) (value metricsMap, ok bool) { + rawValue, ok := metadata.Map(ctx, false).LoadOrStore(keyType{}, metrics) + return rawValue.(metricsMap), ok +} diff --git a/pkg/networkservice/common/metrics/server.go b/pkg/networkservice/common/metrics/server.go index a74be246a..43a4be73b 100644 --- a/pkg/networkservice/common/metrics/server.go +++ b/pkg/networkservice/common/metrics/server.go @@ -31,15 +31,13 @@ import ( ) type metricServer struct { - recorderMap map[string]metric.Int64Histogram - meter metric.Meter + meter metric.Meter } // NewServer returns a new metric server chain element func NewServer() networkservice.NetworkServiceServer { return &metricServer{ - recorderMap: make(map[string]metric.Int64Histogram), - meter: global.Meter(""), + meter: global.Meter(""), } } @@ -70,19 +68,20 @@ func (t *metricServer) writeMetrics(ctx context.Context, path *networkservice.Pa continue } + metrics, _ := loadOrStore(ctx, make(map[string]metric.Int64Histogram)) for metricName, metricValue := range pathSegment.Metrics { /* Works with integers only */ recVal, err := strconv.ParseInt(metricValue, 10, 64) if err != nil { continue } - _, ok := t.recorderMap[metricName] + _, ok := metrics[metricName] if !ok { - t.recorderMap[metricName] = metric.Must(t.meter).NewInt64Histogram( + metrics[metricName] = metric.Must(t.meter).NewInt64Histogram( pathSegment.Name + "_" + metricName, ) } - t.recorderMap[metricName].Record(ctx, recVal, attribute.String("connection", path.GetPathSegments()[0].Id)) + metrics[metricName].Record(ctx, recVal, attribute.String("connection", path.GetPathSegments()[0].Id)) } } } From eac9cc9d53da2e105d1386e1a02e50dace9c6a24 Mon Sep 17 00:00:00 2001 From: Artem Glazychev Date: Thu, 16 Jun 2022 17:43:08 +0700 Subject: [PATCH 2/3] Add unit test Signed-off-by: Artem Glazychev --- .../common/metrics/server_test.go | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 pkg/networkservice/common/metrics/server_test.go diff --git a/pkg/networkservice/common/metrics/server_test.go b/pkg/networkservice/common/metrics/server_test.go new file mode 100644 index 000000000..9bffb40ef --- /dev/null +++ b/pkg/networkservice/common/metrics/server_test.go @@ -0,0 +1,77 @@ +// Copyright (c) 2022 Cisco and/or its affiliates. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed 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 metrics_test + +import ( + "context" + + "math/rand" + "strconv" + "testing" + + "go.uber.org/goleak" + + "github.com/golang/protobuf/ptypes/empty" + "github.com/stretchr/testify/require" + + "github.com/networkservicemesh/api/pkg/api/networkservice" + + "github.com/networkservicemesh/sdk/pkg/networkservice/common/begin" + "github.com/networkservicemesh/sdk/pkg/networkservice/common/metrics" + "github.com/networkservicemesh/sdk/pkg/networkservice/common/updatepath" + "github.com/networkservicemesh/sdk/pkg/networkservice/core/chain" + "github.com/networkservicemesh/sdk/pkg/networkservice/core/next" + "github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata" +) + +func TestMetrics_Concurrency(t *testing.T) { + t.Cleanup(func() { goleak.VerifyNone(t) }) + + server := chain.NewNetworkServiceServer( + begin.NewServer(), + metadata.NewServer(), + updatepath.NewServer("testServer"), + &metricsGeneratorServer{}, + metrics.NewServer(), + ) + for i := 0; i < 100; i++ { + go func(i int) { + req := &networkservice.NetworkServiceRequest{ + Connection: &networkservice.Connection{Id: "nsc-" + strconv.Itoa(i)}, + } + _, err := server.Request(context.Background(), req) + require.NoError(t, err) + }(i) + } +} + +type metricsGeneratorServer struct{} + +func (s *metricsGeneratorServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) { + segment := request.GetConnection().GetPath().GetPathSegments()[0] + if segment.Metrics == nil { + segment.Metrics = make(map[string]string) + } + // Generate any random metric value + // nolint:gosec + segment.Metrics["testMetric"] = strconv.Itoa(rand.Intn(100)) + return next.Server(ctx).Request(ctx, request) +} + +func (s *metricsGeneratorServer) Close(ctx context.Context, connection *networkservice.Connection) (*empty.Empty, error) { + return next.Server(ctx).Close(ctx, connection) +} From b7ab9500edeb44920dc54fa06055f8d28bd79460 Mon Sep 17 00:00:00 2001 From: Artem Glazychev Date: Thu, 16 Jun 2022 18:43:10 +0700 Subject: [PATCH 3/3] fix type alias Signed-off-by: Artem Glazychev --- pkg/networkservice/common/metrics/metadata.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/networkservice/common/metrics/metadata.go b/pkg/networkservice/common/metrics/metadata.go index 788b1ab90..6fe61b251 100644 --- a/pkg/networkservice/common/metrics/metadata.go +++ b/pkg/networkservice/common/metrics/metadata.go @@ -25,7 +25,7 @@ import ( ) type keyType struct{} -type metricsMap map[string]metric.Int64Histogram +type metricsMap = map[string]metric.Int64Histogram func loadOrStore(ctx context.Context, metrics metricsMap) (value metricsMap, ok bool) { rawValue, ok := metadata.Map(ctx, false).LoadOrStore(keyType{}, metrics)