From f1f9b7a9e9e54ae8eff5ef9324bcaf2a438c6649 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Wed, 19 Jun 2024 22:44:04 +0530 Subject: [PATCH 01/41] Instantiated OTEL Metrics Signed-off-by: Wise-Wizard --- pkg/metrics/factory.go | 45 ++++++++++++++++++++++++++++++++++++++ pkg/metrics/otelCounter.go | 28 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 pkg/metrics/otelCounter.go diff --git a/pkg/metrics/factory.go b/pkg/metrics/factory.go index b1b9000a285..1eadfc19a95 100644 --- a/pkg/metrics/factory.go +++ b/pkg/metrics/factory.go @@ -16,7 +16,11 @@ package metrics import ( + "context" "time" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/metric" ) // NSOptions defines the name and tags map associated with a factory namespace @@ -48,6 +52,47 @@ type HistogramOptions struct { Buckets []float64 } +type otelFactory struct { + meter metric.Meter +} + +func NewOTelFactory() Factory { + return &otelFactory{ + meter: otel.Meter("jaeger-V2"), + } +} + +func (f *otelFactory) Counter(options Options) Counter { + counter, _ := f.meter.Int64Counter(options.Name) + attrs := getAttributes(options.Tags) + return &otelCounter{ + counter: counter, + ctx: context.Background(), + attrs: attrs, + } +} + +func (f *otelFactory) Timer(options TimerOptions) Timer { + // Implement the OTEL Timer + return NullTimer +} + +func (f *otelFactory) Gauge(options Options) Gauge { + // Implement the OTEL Gauge + return NullGauge +} + +func (f *otelFactory) Histogram(options HistogramOptions) Histogram { + // Implement the OTEL Histogram + return NullHistogram +} + +func (f *otelFactory) Namespace(scope NSOptions) Factory { + return &otelFactory{ + meter: otel.Meter(scope.Name), + } +} + // Factory creates new metrics type Factory interface { Counter(metric Options) Counter diff --git a/pkg/metrics/otelCounter.go b/pkg/metrics/otelCounter.go new file mode 100644 index 00000000000..6f426111696 --- /dev/null +++ b/pkg/metrics/otelCounter.go @@ -0,0 +1,28 @@ +package metrics + +import ( + "context" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" +) + +// otelCounter is a wrapper around otel.Counter + +type otelCounter struct { + counter metric.Int64Counter + ctx context.Context + attrs []metric.AddOption +} + +func (c *otelCounter) Inc(value int64) { + c.counter.Add(c.ctx, value, c.attrs...) +} + +func getAttributes(tags map[string]string) []metric.AddOption { + var options []metric.AddOption + for k, v := range tags { + options = append(options, metric.WithAttributes(attribute.String(k, v))) + } + return options +} From 12403b18b335440adbeb32237e4df02223418b87 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Wed, 19 Jun 2024 22:57:26 +0530 Subject: [PATCH 02/41] Ran make fmt Signed-off-by: Wise-Wizard --- pkg/metrics/otelCounter.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/metrics/otelCounter.go b/pkg/metrics/otelCounter.go index 6f426111696..4caac6b9224 100644 --- a/pkg/metrics/otelCounter.go +++ b/pkg/metrics/otelCounter.go @@ -1,3 +1,6 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package metrics import ( @@ -10,9 +13,9 @@ import ( // otelCounter is a wrapper around otel.Counter type otelCounter struct { - counter metric.Int64Counter - ctx context.Context - attrs []metric.AddOption + counter metric.Int64Counter + ctx context.Context + attrs []metric.AddOption } func (c *otelCounter) Inc(value int64) { From 0a073b65aff149d46e86f003ca1a4992bf05e0fb Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 21 Jun 2024 14:25:15 +0530 Subject: [PATCH 03/41] Create otelmetrics package Signed-off-by: Wise-Wizard --- pkg/metrics/factory.go | 45 -------------------------- pkg/metrics/otelCounter.go | 31 ------------------ pkg/metrics/otelmetrics/factory.go | 32 ++++++++++++++++++ pkg/metrics/otelmetrics/otelCounter.go | 37 +++++++++++++++++++++ 4 files changed, 69 insertions(+), 76 deletions(-) delete mode 100644 pkg/metrics/otelCounter.go create mode 100644 pkg/metrics/otelmetrics/factory.go create mode 100644 pkg/metrics/otelmetrics/otelCounter.go diff --git a/pkg/metrics/factory.go b/pkg/metrics/factory.go index 1eadfc19a95..b1b9000a285 100644 --- a/pkg/metrics/factory.go +++ b/pkg/metrics/factory.go @@ -16,11 +16,7 @@ package metrics import ( - "context" "time" - - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/metric" ) // NSOptions defines the name and tags map associated with a factory namespace @@ -52,47 +48,6 @@ type HistogramOptions struct { Buckets []float64 } -type otelFactory struct { - meter metric.Meter -} - -func NewOTelFactory() Factory { - return &otelFactory{ - meter: otel.Meter("jaeger-V2"), - } -} - -func (f *otelFactory) Counter(options Options) Counter { - counter, _ := f.meter.Int64Counter(options.Name) - attrs := getAttributes(options.Tags) - return &otelCounter{ - counter: counter, - ctx: context.Background(), - attrs: attrs, - } -} - -func (f *otelFactory) Timer(options TimerOptions) Timer { - // Implement the OTEL Timer - return NullTimer -} - -func (f *otelFactory) Gauge(options Options) Gauge { - // Implement the OTEL Gauge - return NullGauge -} - -func (f *otelFactory) Histogram(options HistogramOptions) Histogram { - // Implement the OTEL Histogram - return NullHistogram -} - -func (f *otelFactory) Namespace(scope NSOptions) Factory { - return &otelFactory{ - meter: otel.Meter(scope.Name), - } -} - // Factory creates new metrics type Factory interface { Counter(metric Options) Counter diff --git a/pkg/metrics/otelCounter.go b/pkg/metrics/otelCounter.go deleted file mode 100644 index 4caac6b9224..00000000000 --- a/pkg/metrics/otelCounter.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) 2024 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package metrics - -import ( - "context" - - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/metric" -) - -// otelCounter is a wrapper around otel.Counter - -type otelCounter struct { - counter metric.Int64Counter - ctx context.Context - attrs []metric.AddOption -} - -func (c *otelCounter) Inc(value int64) { - c.counter.Add(c.ctx, value, c.attrs...) -} - -func getAttributes(tags map[string]string) []metric.AddOption { - var options []metric.AddOption - for k, v := range tags { - options = append(options, metric.WithAttributes(attribute.String(k, v))) - } - return options -} diff --git a/pkg/metrics/otelmetrics/factory.go b/pkg/metrics/otelmetrics/factory.go new file mode 100644 index 00000000000..e57de719a29 --- /dev/null +++ b/pkg/metrics/otelmetrics/factory.go @@ -0,0 +1,32 @@ +package otelmetrics + +import "github.com/jaegertracing/jaeger/pkg/metrics" + +type otelFactory struct{} + +func NewOTELFactory() metrics.Factory { + return &otelFactory{} +} + +func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { + return newOtelCounter(opts.Name, opts.Tags) +} + +func (f *otelFactory) Gauge(opts metrics.Options) metrics.Gauge { + // TODO: Implement OTEL Gauge + return nil +} + +func (f *otelFactory) Timer(opts metrics.TimerOptions) metrics.Timer { + // TODO: Implement OTEL Timer + return nil +} + +func (f *otelFactory) Histogram(opts metrics.HistogramOptions) metrics.Histogram { + // TODO: Implement OTEL Histogram + return nil +} + +func (f *otelFactory) Namespace(opts metrics.NSOptions) metrics.Factory { + return f +} diff --git a/pkg/metrics/otelmetrics/otelCounter.go b/pkg/metrics/otelmetrics/otelCounter.go new file mode 100644 index 00000000000..971071bd3f7 --- /dev/null +++ b/pkg/metrics/otelmetrics/otelCounter.go @@ -0,0 +1,37 @@ +package otelmetrics + +import ( + "context" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" +) + +type otelCounter struct { + counter metric.Int64Counter + attributeSet attribute.Set +} + +func newOtelCounter(name string, tags map[string]string) *otelCounter { + meter := otel.Meter("jaeger-V2") + counter, err := meter.Int64Counter(name) + if err != nil { + panic(err) + } + + attributes := make([]attribute.KeyValue, 0, len(tags)) + for k, v := range tags { + attributes = append(attributes, attribute.String(k, v)) + } + attributeSet := attribute.NewSet(attributes...) + + return &otelCounter{ + counter: counter, + attributeSet: attributeSet, + } +} + +func (c *otelCounter) Inc(value int64) { + c.counter.Add(context.Background(), value, metric.WithAttributeSet(c.attributeSet)) +} From 7efd9b90558cfe32b5c9a3a485c0c81216524140 Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Fri, 21 Jun 2024 19:26:37 +0530 Subject: [PATCH 04/41] Update pkg/metrics/otelmetrics/factory.go Co-authored-by: Yuri Shkuro Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> --- pkg/metrics/otelmetrics/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/metrics/otelmetrics/factory.go b/pkg/metrics/otelmetrics/factory.go index e57de719a29..b4dce7acfe0 100644 --- a/pkg/metrics/otelmetrics/factory.go +++ b/pkg/metrics/otelmetrics/factory.go @@ -4,7 +4,7 @@ import "github.com/jaegertracing/jaeger/pkg/metrics" type otelFactory struct{} -func NewOTELFactory() metrics.Factory { +func NewFactory() metrics.Factory { return &otelFactory{} } From a70d64a6ccd05402ab9f2c4e38ff42c719e141a0 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Sat, 22 Jun 2024 23:06:41 +0530 Subject: [PATCH 05/41] Created benchmark to compare metric implementation Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 32 ++++++++++++++++ .../metrics/otelmetrics/factory.go | 23 +++++++++++- internal/metrics/otelmetrics/otelCounter.go | 17 +++++++++ pkg/metrics/otelmetrics/otelCounter.go | 37 ------------------- 4 files changed, 70 insertions(+), 39 deletions(-) create mode 100644 internal/metrics/benchmark_test.go rename {pkg => internal}/metrics/otelmetrics/factory.go (54%) create mode 100644 internal/metrics/otelmetrics/otelCounter.go delete mode 100644 pkg/metrics/otelmetrics/otelCounter.go diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go new file mode 100644 index 00000000000..b1fe44045c5 --- /dev/null +++ b/internal/metrics/benchmark_test.go @@ -0,0 +1,32 @@ +package benchmark_test + +import ( + "testing" + + "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" + prom "github.com/jaegertracing/jaeger/internal/metrics/prometheus" + "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/prometheus/client_golang/prometheus" +) + +func benchmarkCounter(b *testing.B, factory metrics.Factory) { + counter := factory.Counter(metrics.Options{ + Name: "test_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + + for i := 0; i < b.N; i++ { + counter.Inc(1) + } +} + +func BenchmarkPrometheusCounter(b *testing.B) { + reg := prometheus.NewRegistry() + factory := prom.New(prom.WithRegisterer(reg)) + benchmarkCounter(b, factory) +} + +func BenchmarkOTELCounter(b *testing.B) { + factory := otelmetrics.NewOTELFactory() + benchmarkCounter(b, factory) +} diff --git a/pkg/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go similarity index 54% rename from pkg/metrics/otelmetrics/factory.go rename to internal/metrics/otelmetrics/factory.go index e57de719a29..6bbfe953f74 100644 --- a/pkg/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -1,6 +1,10 @@ package otelmetrics -import "github.com/jaegertracing/jaeger/pkg/metrics" +import ( + "github.com/jaegertracing/jaeger/pkg/metrics" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" +) type otelFactory struct{} @@ -9,7 +13,22 @@ func NewOTELFactory() metrics.Factory { } func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { - return newOtelCounter(opts.Name, opts.Tags) + meter := otel.Meter("jaeger-V2") + counter, err := meter.Int64Counter(opts.Name) + if err != nil { + panic(err) + } + + attributes := make([]attribute.KeyValue, 0, len(opts.Tags)) + for k, v := range opts.Tags { + attributes = append(attributes, attribute.String(k, v)) + } + attributeSet := attribute.NewSet(attributes...) + + return &otelCounter{ + counter: counter, + attributeSet: attributeSet, + } } func (f *otelFactory) Gauge(opts metrics.Options) metrics.Gauge { diff --git a/internal/metrics/otelmetrics/otelCounter.go b/internal/metrics/otelmetrics/otelCounter.go new file mode 100644 index 00000000000..67a41b36273 --- /dev/null +++ b/internal/metrics/otelmetrics/otelCounter.go @@ -0,0 +1,17 @@ +package otelmetrics + +import ( + "context" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" +) + +type otelCounter struct { + counter metric.Int64Counter + attributeSet attribute.Set +} + +func (c *otelCounter) Inc(value int64) { + c.counter.Add(context.Background(), value, metric.WithAttributeSet(c.attributeSet)) +} diff --git a/pkg/metrics/otelmetrics/otelCounter.go b/pkg/metrics/otelmetrics/otelCounter.go deleted file mode 100644 index 971071bd3f7..00000000000 --- a/pkg/metrics/otelmetrics/otelCounter.go +++ /dev/null @@ -1,37 +0,0 @@ -package otelmetrics - -import ( - "context" - - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/metric" -) - -type otelCounter struct { - counter metric.Int64Counter - attributeSet attribute.Set -} - -func newOtelCounter(name string, tags map[string]string) *otelCounter { - meter := otel.Meter("jaeger-V2") - counter, err := meter.Int64Counter(name) - if err != nil { - panic(err) - } - - attributes := make([]attribute.KeyValue, 0, len(tags)) - for k, v := range tags { - attributes = append(attributes, attribute.String(k, v)) - } - attributeSet := attribute.NewSet(attributes...) - - return &otelCounter{ - counter: counter, - attributeSet: attributeSet, - } -} - -func (c *otelCounter) Inc(value int64) { - c.counter.Add(context.Background(), value, metric.WithAttributeSet(c.attributeSet)) -} From dc2a9edfe5c398a64fe7003f3ff9e4f92dd5365e Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Sat, 22 Jun 2024 23:08:59 +0530 Subject: [PATCH 06/41] Resolved conflicts Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index b1fe44045c5..6ee2e5ed131 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -27,6 +27,6 @@ func BenchmarkPrometheusCounter(b *testing.B) { } func BenchmarkOTELCounter(b *testing.B) { - factory := otelmetrics.NewOTELFactory() + factory := otelmetrics.NewFactory() benchmarkCounter(b, factory) } From 4a9929151f665d36103bf8a0df0a2f3f43bd0be4 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sat, 22 Jun 2024 16:27:26 -0400 Subject: [PATCH 07/41] avoid allocations tested via ``` $ go test -benchmem -benchtime=5s -bench=Benchmark ./internal/metrics/ ``` before: ``` BenchmarkPrometheusCounter-10 856818336 6.875 ns/op 0 B/op 0 allocs/op BenchmarkOTELCounter-10 146044255 40.92 ns/op 32 B/op 2 allocs/op ``` after: `` BenchmarkPrometheusCounter-10 855046669 6.924 ns/op 0 B/op 0 allocs/op BenchmarkOTELCounter-10 293330721 21.05 ns/op 16 B/op 1 allocs/op ``` Signed-off-by: Yuri Shkuro --- internal/metrics/otelmetrics/factory.go | 11 ++++++++--- internal/metrics/otelmetrics/otelCounter.go | 8 ++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index ab846c2e43f..6d76dbeb8b8 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -1,9 +1,13 @@ package otelmetrics import ( - "github.com/jaegertracing/jaeger/pkg/metrics" + "context" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + + "github.com/jaegertracing/jaeger/pkg/metrics" ) type otelFactory struct{} @@ -26,8 +30,9 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { attributeSet := attribute.NewSet(attributes...) return &otelCounter{ - counter: counter, - attributeSet: attributeSet, + counter: counter, + fixedCtx: context.Background(), + option: metric.WithAttributeSet(attributeSet), } } diff --git a/internal/metrics/otelmetrics/otelCounter.go b/internal/metrics/otelmetrics/otelCounter.go index 67a41b36273..c66e41c2bdf 100644 --- a/internal/metrics/otelmetrics/otelCounter.go +++ b/internal/metrics/otelmetrics/otelCounter.go @@ -3,15 +3,15 @@ package otelmetrics import ( "context" - "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" ) type otelCounter struct { - counter metric.Int64Counter - attributeSet attribute.Set + counter metric.Int64Counter + fixedCtx context.Context + option metric.AddOption } func (c *otelCounter) Inc(value int64) { - c.counter.Add(context.Background(), value, metric.WithAttributeSet(c.attributeSet)) + c.counter.Add(c.fixedCtx, value, c.option) } From 099ba05d5ed5c29b39a8afebda4d51b885274555 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Sun, 23 Jun 2024 02:14:25 +0530 Subject: [PATCH 08/41] Implemented changes Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 6 +++++- .../metrics/otelmetrics/{otelCounter.go => counter.go} | 7 +++++++ internal/metrics/otelmetrics/factory.go | 7 ++++++- 3 files changed, 18 insertions(+), 2 deletions(-) rename internal/metrics/otelmetrics/{otelCounter.go => counter.go} (64%) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index 6ee2e5ed131..f7d5eadf606 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -1,12 +1,16 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package benchmark_test import ( "testing" + "github.com/prometheus/client_golang/prometheus" + "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" prom "github.com/jaegertracing/jaeger/internal/metrics/prometheus" "github.com/jaegertracing/jaeger/pkg/metrics" - "github.com/prometheus/client_golang/prometheus" ) func benchmarkCounter(b *testing.B, factory metrics.Factory) { diff --git a/internal/metrics/otelmetrics/otelCounter.go b/internal/metrics/otelmetrics/counter.go similarity index 64% rename from internal/metrics/otelmetrics/otelCounter.go rename to internal/metrics/otelmetrics/counter.go index c66e41c2bdf..eaaf8c0d966 100644 --- a/internal/metrics/otelmetrics/otelCounter.go +++ b/internal/metrics/otelmetrics/counter.go @@ -1,3 +1,6 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package otelmetrics import ( @@ -15,3 +18,7 @@ type otelCounter struct { func (c *otelCounter) Inc(value int64) { c.counter.Add(c.fixedCtx, value, c.option) } + +type noopCounter struct{} + +func (c *noopCounter) Inc(value int64) {} \ No newline at end of file diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index 6d76dbeb8b8..3d58725bd9c 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -1,7 +1,11 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package otelmetrics import ( "context" + "log" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -20,7 +24,8 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { meter := otel.Meter("jaeger-V2") counter, err := meter.Int64Counter(opts.Name) if err != nil { - panic(err) + log.Printf("Error creating OTEL counter: %v", err) + return &noopCounter{} } attributes := make([]attribute.KeyValue, 0, len(opts.Tags)) From 8b47b1227ef24a3ec0ed5610037045609ed81191 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Sun, 23 Jun 2024 17:20:35 +0530 Subject: [PATCH 09/41] Used metrics.NullCounter as return type Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/counter.go | 4 ---- internal/metrics/otelmetrics/factory.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/metrics/otelmetrics/counter.go b/internal/metrics/otelmetrics/counter.go index eaaf8c0d966..437442306f3 100644 --- a/internal/metrics/otelmetrics/counter.go +++ b/internal/metrics/otelmetrics/counter.go @@ -18,7 +18,3 @@ type otelCounter struct { func (c *otelCounter) Inc(value int64) { c.counter.Add(c.fixedCtx, value, c.option) } - -type noopCounter struct{} - -func (c *noopCounter) Inc(value int64) {} \ No newline at end of file diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index 3d58725bd9c..d596d558d57 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -25,7 +25,7 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { counter, err := meter.Int64Counter(opts.Name) if err != nil { log.Printf("Error creating OTEL counter: %v", err) - return &noopCounter{} + return metrics.NullCounter } attributes := make([]attribute.KeyValue, 0, len(opts.Tags)) From 8032ce222db94be81aa4440b88dd4d369482b30a Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Sun, 23 Jun 2024 18:16:57 +0530 Subject: [PATCH 10/41] Created rough implementation of OTEL SDK Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory.go | 32 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index d596d558d57..6b2d298ee77 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -7,22 +7,40 @@ import ( "context" "log" + "github.com/jaegertracing/jaeger/pkg/metrics" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/metric" - - "github.com/jaegertracing/jaeger/pkg/metrics" + metric "go.opentelemetry.io/otel/metric" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/resource" + semconv "go.opentelemetry.io/otel/semconv/v1.4.0" ) -type otelFactory struct{} +type otelFactory struct { + meter metric.Meter +} func NewFactory() metrics.Factory { - return &otelFactory{} + res, err := resource.New(context.Background(), + resource.WithAttributes( + semconv.ServiceNameKey.String("jaeger-V2"), + ), + ) + if err != nil { + log.Fatalf("Could not create resource: %v", err) + } + meterProvider := sdkmetric.NewMeterProvider( + sdkmetric.WithResource(res), + sdkmetric.WithReader(sdkmetric.NewManualReader()), + ) + otel.SetMeterProvider(meterProvider) + return &otelFactory{ + meter: otel.Meter("jaeger-V2"), + } } func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { - meter := otel.Meter("jaeger-V2") - counter, err := meter.Int64Counter(opts.Name) + counter, err := f.meter.Int64Counter(opts.Name) if err != nil { log.Printf("Error creating OTEL counter: %v", err) return metrics.NullCounter From 6ec5ec4b67136cf01a98508c14855c70951c23a9 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Mon, 24 Jun 2024 12:30:35 +0530 Subject: [PATCH 11/41] Fixed initialization errors Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 8 +++++++- internal/metrics/otelmetrics/factory.go | 16 +--------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index f7d5eadf606..74fd4ca353c 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/prometheus/client_golang/prometheus" + "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/resource" "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" prom "github.com/jaegertracing/jaeger/internal/metrics/prometheus" @@ -31,6 +33,10 @@ func BenchmarkPrometheusCounter(b *testing.B) { } func BenchmarkOTELCounter(b *testing.B) { - factory := otelmetrics.NewFactory() + res := resource.NewWithAttributes( + resource.Default().SchemaURL(), + ) + meterProvider := metric.NewMeterProvider(metric.WithResource(res)) + factory := otelmetrics.NewFactory(meterProvider) benchmarkCounter(b, factory) } diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index 6b2d298ee77..e543c0abe59 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -12,27 +12,13 @@ import ( "go.opentelemetry.io/otel/attribute" metric "go.opentelemetry.io/otel/metric" sdkmetric "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/resource" - semconv "go.opentelemetry.io/otel/semconv/v1.4.0" ) type otelFactory struct { meter metric.Meter } -func NewFactory() metrics.Factory { - res, err := resource.New(context.Background(), - resource.WithAttributes( - semconv.ServiceNameKey.String("jaeger-V2"), - ), - ) - if err != nil { - log.Fatalf("Could not create resource: %v", err) - } - meterProvider := sdkmetric.NewMeterProvider( - sdkmetric.WithResource(res), - sdkmetric.WithReader(sdkmetric.NewManualReader()), - ) +func NewFactory(meterProvider *sdkmetric.MeterProvider) metrics.Factory { otel.SetMeterProvider(meterProvider) return &otelFactory{ meter: otel.Meter("jaeger-V2"), From 4e7dcbbd37f4754bd7362fa170ec8059706fdcdd Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Mon, 24 Jun 2024 22:33:17 +0530 Subject: [PATCH 12/41] Made suggested changes Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 6 +----- internal/metrics/otelmetrics/factory.go | 7 ++----- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index 74fd4ca353c..fa176512e13 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -8,7 +8,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/resource" "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" prom "github.com/jaegertracing/jaeger/internal/metrics/prometheus" @@ -33,10 +32,7 @@ func BenchmarkPrometheusCounter(b *testing.B) { } func BenchmarkOTELCounter(b *testing.B) { - res := resource.NewWithAttributes( - resource.Default().SchemaURL(), - ) - meterProvider := metric.NewMeterProvider(metric.WithResource(res)) + meterProvider := metric.NewMeterProvider() factory := otelmetrics.NewFactory(meterProvider) benchmarkCounter(b, factory) } diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index e543c0abe59..6f1ab738c92 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -8,20 +8,17 @@ import ( "log" "github.com/jaegertracing/jaeger/pkg/metrics" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" metric "go.opentelemetry.io/otel/metric" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" ) type otelFactory struct { meter metric.Meter } -func NewFactory(meterProvider *sdkmetric.MeterProvider) metrics.Factory { - otel.SetMeterProvider(meterProvider) +func NewFactory(meterProvider metric.MeterProvider) metrics.Factory { return &otelFactory{ - meter: otel.Meter("jaeger-V2"), + meter: meterProvider.Meter("jaeger-v2"), } } From f29cad5cac4315a2f30148929c62a6be860fc920 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Tue, 25 Jun 2024 00:21:09 +0530 Subject: [PATCH 13/41] Created Prometheus Exporter Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index fa176512e13..47336ce02be 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -4,14 +4,16 @@ package benchmark_test import ( + "log" "testing" - "github.com/prometheus/client_golang/prometheus" + prometheus "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/sdk/metric" "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" prom "github.com/jaegertracing/jaeger/internal/metrics/prometheus" "github.com/jaegertracing/jaeger/pkg/metrics" + promExporter "go.opentelemetry.io/otel/exporters/prometheus" ) func benchmarkCounter(b *testing.B, factory metrics.Factory) { @@ -32,7 +34,14 @@ func BenchmarkPrometheusCounter(b *testing.B) { } func BenchmarkOTELCounter(b *testing.B) { - meterProvider := metric.NewMeterProvider() + registry := prometheus.NewRegistry() + exporter, err := promExporter.New(promExporter.WithRegisterer(registry)) + if err != nil { + log.Fatalf("Failed to create Prometheus exporter: %v", err) + } + meterProvider := metric.NewMeterProvider( + metric.WithReader(exporter), + ) factory := otelmetrics.NewFactory(meterProvider) benchmarkCounter(b, factory) } From 67e23e263c1c5ef2aac995cba5be03384f2f1da4 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Tue, 25 Jun 2024 19:51:29 +0530 Subject: [PATCH 14/41] Implemented OTEL Metrics for other Instruments Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 94 ++++++++++++++++++----- internal/metrics/otelmetrics/factory.go | 64 +++++++++++---- internal/metrics/otelmetrics/gauge.go | 20 +++++ internal/metrics/otelmetrics/histogram.go | 20 +++++ internal/metrics/otelmetrics/timer.go | 21 +++++ 5 files changed, 186 insertions(+), 33 deletions(-) create mode 100644 internal/metrics/otelmetrics/gauge.go create mode 100644 internal/metrics/otelmetrics/histogram.go create mode 100644 internal/metrics/otelmetrics/timer.go diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index 47336ce02be..07e23d7b8fc 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -8,32 +8,20 @@ import ( "testing" prometheus "github.com/prometheus/client_golang/prometheus" + promExporter "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" prom "github.com/jaegertracing/jaeger/internal/metrics/prometheus" "github.com/jaegertracing/jaeger/pkg/metrics" - promExporter "go.opentelemetry.io/otel/exporters/prometheus" ) -func benchmarkCounter(b *testing.B, factory metrics.Factory) { - counter := factory.Counter(metrics.Options{ - Name: "test_counter", - Tags: map[string]string{"tag1": "value1"}, - }) - - for i := 0; i < b.N; i++ { - counter.Inc(1) - } -} - -func BenchmarkPrometheusCounter(b *testing.B) { +func setupPrometheusFactory() metrics.Factory { reg := prometheus.NewRegistry() - factory := prom.New(prom.WithRegisterer(reg)) - benchmarkCounter(b, factory) + return prom.New(prom.WithRegisterer(reg)) } -func BenchmarkOTELCounter(b *testing.B) { +func setupOTELFactory() metrics.Factory { registry := prometheus.NewRegistry() exporter, err := promExporter.New(promExporter.WithRegisterer(registry)) if err != nil { @@ -42,6 +30,76 @@ func BenchmarkOTELCounter(b *testing.B) { meterProvider := metric.NewMeterProvider( metric.WithReader(exporter), ) - factory := otelmetrics.NewFactory(meterProvider) - benchmarkCounter(b, factory) + return otelmetrics.NewFactory(meterProvider) +} + +func benchmarkMetric(b *testing.B, factory metrics.Factory, metricType string) { + var metricObj interface{} + switch metricType { + case "counter": + metricObj = factory.Counter(metrics.Options{ + Name: "test_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + case "gauge": + metricObj = factory.Gauge(metrics.Options{ + Name: "test_gauge", + Tags: map[string]string{"tag1": "value1"}, + }) + case "timer": + metricObj = factory.Timer(metrics.TimerOptions{ + Name: "test_timer", + Tags: map[string]string{"tag1": "value1"}, + }) + case "histogram": + metricObj = factory.Histogram(metrics.HistogramOptions{ + Name: "test_histogram", + Tags: map[string]string{"tag1": "value1"}, + }) + } + + for i := 0; i < b.N; i++ { + switch m := metricObj.(type) { + case metrics.Counter: + m.Inc(1) + case metrics.Gauge: + m.Update(1) + case metrics.Timer: + m.Record(100) + case metrics.Histogram: + m.Record(1.0) + } + } +} + +func BenchmarkPrometheusCounter(b *testing.B) { + benchmarkMetric(b, setupPrometheusFactory(), "counter") +} + +func BenchmarkOTELCounter(b *testing.B) { + benchmarkMetric(b, setupOTELFactory(), "counter") +} + +func BenchmarkPrometheusGauge(b *testing.B) { + benchmarkMetric(b, setupPrometheusFactory(), "gauge") +} + +func BenchmarkOTELGauge(b *testing.B) { + benchmarkMetric(b, setupOTELFactory(), "gauge") +} + +func BenchmarkPrometheusTimer(b *testing.B) { + benchmarkMetric(b, setupPrometheusFactory(), "timer") +} + +func BenchmarkOTELTimer(b *testing.B) { + benchmarkMetric(b, setupOTELFactory(), "timer") +} + +func BenchmarkPrometheusHistogram(b *testing.B) { + benchmarkMetric(b, setupPrometheusFactory(), "histogram") +} + +func BenchmarkOTELHistogram(b *testing.B) { + benchmarkMetric(b, setupOTELFactory(), "histogram") } diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index 6f1ab738c92..3a3fe98608a 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -7,9 +7,10 @@ import ( "context" "log" - "github.com/jaegertracing/jaeger/pkg/metrics" "go.opentelemetry.io/otel/attribute" metric "go.opentelemetry.io/otel/metric" + + "github.com/jaegertracing/jaeger/pkg/metrics" ) type otelFactory struct { @@ -29,12 +30,7 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { return metrics.NullCounter } - attributes := make([]attribute.KeyValue, 0, len(opts.Tags)) - for k, v := range opts.Tags { - attributes = append(attributes, attribute.String(k, v)) - } - attributeSet := attribute.NewSet(attributes...) - + attributeSet := f.CreateAtrributeSet(opts.Tags) return &otelCounter{ counter: counter, fixedCtx: context.Background(), @@ -43,20 +39,58 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { } func (f *otelFactory) Gauge(opts metrics.Options) metrics.Gauge { - // TODO: Implement OTEL Gauge - return nil -} + gauge, err := f.meter.Int64Gauge(opts.Name) + if err != nil { + log.Printf("Error creating OTEL gauge: %v", err) + return metrics.NullGauge -func (f *otelFactory) Timer(opts metrics.TimerOptions) metrics.Timer { - // TODO: Implement OTEL Timer - return nil + } + + attributeSet := f.CreateAtrributeSet(opts.Tags) + return &otelGauge{ + gauge: gauge, + fixedCtx: context.Background(), + option: metric.WithAttributeSet(attributeSet), + } } func (f *otelFactory) Histogram(opts metrics.HistogramOptions) metrics.Histogram { - // TODO: Implement OTEL Histogram - return nil + histogram, err := f.meter.Float64Histogram(opts.Name) + if err != nil { + log.Printf("Error creating OTEL histogram: %v", err) + return metrics.NullHistogram + } + + attributeSet := f.CreateAtrributeSet(opts.Tags) + return &otelHistogram{ + histogram: histogram, + fixedCtx: context.Background(), + option: metric.WithAttributeSet(attributeSet), + } +} + +func (f *otelFactory) Timer(opts metrics.TimerOptions) metrics.Timer { + timer, err := f.meter.Float64Histogram(opts.Name) + if err != nil { + log.Printf("Error creating OTEL timer: %v", err) + return metrics.NullTimer + } + attributeSet := f.CreateAtrributeSet(opts.Tags) + return &otelTimer{ + histogram: timer, + fixedCtx: context.Background(), + option: metric.WithAttributeSet(attributeSet), + } } func (f *otelFactory) Namespace(opts metrics.NSOptions) metrics.Factory { return f } + +func (f *otelFactory) CreateAtrributeSet(tags map[string]string) attribute.Set { + attributes := make([]attribute.KeyValue, 0, len(tags)) + for k, v := range tags { + attributes = append(attributes, attribute.String(k, v)) + } + return attribute.NewSet(attributes...) +} diff --git a/internal/metrics/otelmetrics/gauge.go b/internal/metrics/otelmetrics/gauge.go new file mode 100644 index 00000000000..dc5c4f38428 --- /dev/null +++ b/internal/metrics/otelmetrics/gauge.go @@ -0,0 +1,20 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package otelmetrics + +import ( + "context" + + "go.opentelemetry.io/otel/metric" +) + +type otelGauge struct { + gauge metric.Int64Gauge + fixedCtx context.Context + option metric.RecordOption +} + +func (g *otelGauge) Update(value int64) { + g.gauge.Record(g.fixedCtx, value, g.option) +} diff --git a/internal/metrics/otelmetrics/histogram.go b/internal/metrics/otelmetrics/histogram.go new file mode 100644 index 00000000000..b408b47dd7c --- /dev/null +++ b/internal/metrics/otelmetrics/histogram.go @@ -0,0 +1,20 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package otelmetrics + +import ( + "context" + + "go.opentelemetry.io/otel/metric" +) + +type otelHistogram struct { + histogram metric.Float64Histogram + fixedCtx context.Context + option metric.RecordOption +} + +func (h *otelHistogram) Record(value float64) { + h.histogram.Record(h.fixedCtx, value, h.option) +} diff --git a/internal/metrics/otelmetrics/timer.go b/internal/metrics/otelmetrics/timer.go new file mode 100644 index 00000000000..0df689243b3 --- /dev/null +++ b/internal/metrics/otelmetrics/timer.go @@ -0,0 +1,21 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package otelmetrics + +import ( + "context" + "time" + + "go.opentelemetry.io/otel/metric" +) + +type otelTimer struct { + histogram metric.Float64Histogram + fixedCtx context.Context + option metric.RecordOption +} + +func (t *otelTimer) Record(d time.Duration) { + t.histogram.Record(t.fixedCtx, d.Seconds(), t.option) +} From 598c39fc3343724b34aae0cf5fedb11687af9201 Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Tue, 25 Jun 2024 21:48:21 +0530 Subject: [PATCH 15/41] Update internal/metrics/otelmetrics/factory.go Co-authored-by: Yuri Shkuro Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> --- internal/metrics/otelmetrics/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index 3a3fe98608a..a1d45cc8530 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -34,7 +34,7 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { return &otelCounter{ counter: counter, fixedCtx: context.Background(), - option: metric.WithAttributeSet(attributeSet), + option: attributeSetOption(opts.Tags), } } From 29f63826b69e9761c7051fbd33ec04e76c3434fe Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Tue, 25 Jun 2024 21:50:00 +0530 Subject: [PATCH 16/41] Update internal/metrics/benchmark_test.go Co-authored-by: Yuri Shkuro Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> --- internal/metrics/benchmark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index 07e23d7b8fc..e43d3e07a52 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -21,7 +21,7 @@ func setupPrometheusFactory() metrics.Factory { return prom.New(prom.WithRegisterer(reg)) } -func setupOTELFactory() metrics.Factory { +func setupOTELFactory(b *testing.B) metrics.Factory { registry := prometheus.NewRegistry() exporter, err := promExporter.New(promExporter.WithRegisterer(registry)) if err != nil { From 51bd2f0f9b962916abe1e81a1c36b52afe976f13 Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Tue, 25 Jun 2024 21:50:16 +0530 Subject: [PATCH 17/41] Update internal/metrics/benchmark_test.go Co-authored-by: Yuri Shkuro Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> --- internal/metrics/benchmark_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index e43d3e07a52..a96545aa5c7 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -24,9 +24,7 @@ func setupPrometheusFactory() metrics.Factory { func setupOTELFactory(b *testing.B) metrics.Factory { registry := prometheus.NewRegistry() exporter, err := promExporter.New(promExporter.WithRegisterer(registry)) - if err != nil { - log.Fatalf("Failed to create Prometheus exporter: %v", err) - } + require.NoError(b, err) meterProvider := metric.NewMeterProvider( metric.WithReader(exporter), ) From 22fe680f06a12e88f66d1eb7572e9e0d16ad7776 Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Tue, 25 Jun 2024 21:50:40 +0530 Subject: [PATCH 18/41] Update internal/metrics/otelmetrics/factory.go Co-authored-by: Yuri Shkuro Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> --- internal/metrics/otelmetrics/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index a1d45cc8530..9dd6fa582c0 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -87,7 +87,7 @@ func (f *otelFactory) Namespace(opts metrics.NSOptions) metrics.Factory { return f } -func (f *otelFactory) CreateAtrributeSet(tags map[string]string) attribute.Set { +func attributeSetOption(tags map[string]string) metric.Option { attributes := make([]attribute.KeyValue, 0, len(tags)) for k, v := range tags { attributes = append(attributes, attribute.String(k, v)) From 6f41a526b743b0fd9c128de2c5b95497d324ef18 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Tue, 25 Jun 2024 22:05:45 +0530 Subject: [PATCH 19/41] Refactored Code Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 89 +++++++++++++------------ internal/metrics/otelmetrics/factory.go | 15 ++--- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index a96545aa5c7..73d133adbe0 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -4,10 +4,10 @@ package benchmark_test import ( - "log" "testing" prometheus "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" promExporter "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" @@ -31,73 +31,78 @@ func setupOTELFactory(b *testing.B) metrics.Factory { return otelmetrics.NewFactory(meterProvider) } -func benchmarkMetric(b *testing.B, factory metrics.Factory, metricType string) { - var metricObj interface{} - switch metricType { - case "counter": - metricObj = factory.Counter(metrics.Options{ - Name: "test_counter", - Tags: map[string]string{"tag1": "value1"}, - }) - case "gauge": - metricObj = factory.Gauge(metrics.Options{ - Name: "test_gauge", - Tags: map[string]string{"tag1": "value1"}, - }) - case "timer": - metricObj = factory.Timer(metrics.TimerOptions{ - Name: "test_timer", - Tags: map[string]string{"tag1": "value1"}, - }) - case "histogram": - metricObj = factory.Histogram(metrics.HistogramOptions{ - Name: "test_histogram", - Tags: map[string]string{"tag1": "value1"}, - }) +func benchmarkCounter(b *testing.B, factory metrics.Factory) { + counter := factory.Counter(metrics.Options{ + Name: "test_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + + for i := 0; i < b.N; i++ { + counter.Inc(1) + } +} + +func benchmarkGauge(b *testing.B, factory metrics.Factory) { + gauge := factory.Gauge(metrics.Options{ + Name: "test_gauge", + Tags: map[string]string{"tag1": "value1"}, + }) + + for i := 0; i < b.N; i++ { + gauge.Update(1) } +} + +func benchmarkTimer(b *testing.B, factory metrics.Factory) { + timer := factory.Timer(metrics.TimerOptions{ + Name: "test_timer", + Tags: map[string]string{"tag1": "value1"}, + }) + + for i := 0; i < b.N; i++ { + timer.Record(100) + } +} + +func benchmarkHistogram(b *testing.B, factory metrics.Factory) { + histogram := factory.Histogram(metrics.HistogramOptions{ + Name: "test_histogram", + Tags: map[string]string{"tag1": "value1"}, + }) for i := 0; i < b.N; i++ { - switch m := metricObj.(type) { - case metrics.Counter: - m.Inc(1) - case metrics.Gauge: - m.Update(1) - case metrics.Timer: - m.Record(100) - case metrics.Histogram: - m.Record(1.0) - } + histogram.Record(1.0) } } func BenchmarkPrometheusCounter(b *testing.B) { - benchmarkMetric(b, setupPrometheusFactory(), "counter") + benchmarkCounter(b, setupPrometheusFactory()) } func BenchmarkOTELCounter(b *testing.B) { - benchmarkMetric(b, setupOTELFactory(), "counter") + benchmarkCounter(b, setupOTELFactory(b)) } func BenchmarkPrometheusGauge(b *testing.B) { - benchmarkMetric(b, setupPrometheusFactory(), "gauge") + benchmarkGauge(b, setupPrometheusFactory()) } func BenchmarkOTELGauge(b *testing.B) { - benchmarkMetric(b, setupOTELFactory(), "gauge") + benchmarkGauge(b, setupOTELFactory(b)) } func BenchmarkPrometheusTimer(b *testing.B) { - benchmarkMetric(b, setupPrometheusFactory(), "timer") + benchmarkTimer(b, setupPrometheusFactory()) } func BenchmarkOTELTimer(b *testing.B) { - benchmarkMetric(b, setupOTELFactory(), "timer") + benchmarkTimer(b, setupOTELFactory(b)) } func BenchmarkPrometheusHistogram(b *testing.B) { - benchmarkMetric(b, setupPrometheusFactory(), "histogram") + benchmarkHistogram(b, setupPrometheusFactory()) } func BenchmarkOTELHistogram(b *testing.B) { - benchmarkMetric(b, setupOTELFactory(), "histogram") + benchmarkHistogram(b, setupOTELFactory(b)) } diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index 9dd6fa582c0..e428ef47c3d 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -29,8 +29,6 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { log.Printf("Error creating OTEL counter: %v", err) return metrics.NullCounter } - - attributeSet := f.CreateAtrributeSet(opts.Tags) return &otelCounter{ counter: counter, fixedCtx: context.Background(), @@ -46,11 +44,10 @@ func (f *otelFactory) Gauge(opts metrics.Options) metrics.Gauge { } - attributeSet := f.CreateAtrributeSet(opts.Tags) return &otelGauge{ gauge: gauge, fixedCtx: context.Background(), - option: metric.WithAttributeSet(attributeSet), + option: attributeSetOption(opts.Tags), } } @@ -61,11 +58,10 @@ func (f *otelFactory) Histogram(opts metrics.HistogramOptions) metrics.Histogram return metrics.NullHistogram } - attributeSet := f.CreateAtrributeSet(opts.Tags) return &otelHistogram{ histogram: histogram, fixedCtx: context.Background(), - option: metric.WithAttributeSet(attributeSet), + option: attributeSetOption(opts.Tags), } } @@ -75,11 +71,10 @@ func (f *otelFactory) Timer(opts metrics.TimerOptions) metrics.Timer { log.Printf("Error creating OTEL timer: %v", err) return metrics.NullTimer } - attributeSet := f.CreateAtrributeSet(opts.Tags) return &otelTimer{ histogram: timer, fixedCtx: context.Background(), - option: metric.WithAttributeSet(attributeSet), + option: attributeSetOption(opts.Tags), } } @@ -87,10 +82,10 @@ func (f *otelFactory) Namespace(opts metrics.NSOptions) metrics.Factory { return f } -func attributeSetOption(tags map[string]string) metric.Option { +func attributeSetOption(tags map[string]string) metric.MeasurementOption { attributes := make([]attribute.KeyValue, 0, len(tags)) for k, v := range tags { attributes = append(attributes, attribute.String(k, v)) } - return attribute.NewSet(attributes...) + return metric.WithAttributes(attributes...) } From a6212bbc3cd0634c085659b6d82f40fca70e308f Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Tue, 25 Jun 2024 22:23:21 +0530 Subject: [PATCH 20/41] Added Unit Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index e428ef47c3d..6e244bb93d5 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -66,7 +66,7 @@ func (f *otelFactory) Histogram(opts metrics.HistogramOptions) metrics.Histogram } func (f *otelFactory) Timer(opts metrics.TimerOptions) metrics.Timer { - timer, err := f.meter.Float64Histogram(opts.Name) + timer, err := f.meter.Float64Histogram(opts.Name, metric.WithUnit("s")) if err != nil { log.Printf("Error creating OTEL timer: %v", err) return metrics.NullTimer From d6347c440c10f8002f997f392b06124190d0eac4 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Wed, 26 Jun 2024 18:36:44 +0530 Subject: [PATCH 21/41] Added Namespace Configuration Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory.go | 68 ++++++++++++++++++++----- 1 file changed, 55 insertions(+), 13 deletions(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index 6e244bb93d5..d14e60c616c 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -6,25 +6,34 @@ package otelmetrics import ( "context" "log" + "strings" "go.opentelemetry.io/otel/attribute" - metric "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric" "github.com/jaegertracing/jaeger/pkg/metrics" ) type otelFactory struct { - meter metric.Meter + meter metric.Meter + scope string + separator string + normalizer *strings.Replacer + tags map[string]string } func NewFactory(meterProvider metric.MeterProvider) metrics.Factory { return &otelFactory{ - meter: meterProvider.Meter("jaeger-v2"), + meter: meterProvider.Meter("jaeger-v2"), + separator: ".", + normalizer: strings.NewReplacer(" ", "_", ".", "_", "-", "_"), + tags: make(map[string]string), } } func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { - counter, err := f.meter.Int64Counter(opts.Name) + name := f.subScope(opts.Name) + counter, err := f.meter.Int64Counter(name) if err != nil { log.Printf("Error creating OTEL counter: %v", err) return metrics.NullCounter @@ -32,27 +41,28 @@ func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { return &otelCounter{ counter: counter, fixedCtx: context.Background(), - option: attributeSetOption(opts.Tags), + option: attributeSetOption(f.mergeTags(opts.Tags)), } } func (f *otelFactory) Gauge(opts metrics.Options) metrics.Gauge { - gauge, err := f.meter.Int64Gauge(opts.Name) + name := f.subScope(opts.Name) + gauge, err := f.meter.Int64Gauge(name) if err != nil { log.Printf("Error creating OTEL gauge: %v", err) return metrics.NullGauge - } return &otelGauge{ gauge: gauge, fixedCtx: context.Background(), - option: attributeSetOption(opts.Tags), + option: attributeSetOption(f.mergeTags(opts.Tags)), } } func (f *otelFactory) Histogram(opts metrics.HistogramOptions) metrics.Histogram { - histogram, err := f.meter.Float64Histogram(opts.Name) + name := f.subScope(opts.Name) + histogram, err := f.meter.Float64Histogram(name) if err != nil { log.Printf("Error creating OTEL histogram: %v", err) return metrics.NullHistogram @@ -61,12 +71,13 @@ func (f *otelFactory) Histogram(opts metrics.HistogramOptions) metrics.Histogram return &otelHistogram{ histogram: histogram, fixedCtx: context.Background(), - option: attributeSetOption(opts.Tags), + option: attributeSetOption(f.mergeTags(opts.Tags)), } } func (f *otelFactory) Timer(opts metrics.TimerOptions) metrics.Timer { - timer, err := f.meter.Float64Histogram(opts.Name, metric.WithUnit("s")) + name := f.subScope(opts.Name) + timer, err := f.meter.Float64Histogram(name, metric.WithUnit("s")) if err != nil { log.Printf("Error creating OTEL timer: %v", err) return metrics.NullTimer @@ -74,12 +85,43 @@ func (f *otelFactory) Timer(opts metrics.TimerOptions) metrics.Timer { return &otelTimer{ histogram: timer, fixedCtx: context.Background(), - option: attributeSetOption(opts.Tags), + option: attributeSetOption(f.mergeTags(opts.Tags)), } } func (f *otelFactory) Namespace(opts metrics.NSOptions) metrics.Factory { - return f + return &otelFactory{ + meter: f.meter, + scope: f.subScope(opts.Name), + separator: f.separator, + normalizer: f.normalizer, + tags: f.mergeTags(opts.Tags), + } +} + +func (f *otelFactory) subScope(name string) string { + if f.scope == "" { + return f.normalize(name) + } + if name == "" { + return f.normalize(f.scope) + } + return f.normalize(f.scope + f.separator + name) +} + +func (f *otelFactory) normalize(v string) string { + return f.normalizer.Replace(v) +} + +func (f *otelFactory) mergeTags(tags map[string]string) map[string]string { + merged := make(map[string]string) + for k, v := range f.tags { + merged[k] = v + } + for k, v := range tags { + merged[k] = v + } + return merged } func attributeSetOption(tags map[string]string) metric.MeasurementOption { From 1c667f2cd8a9ace1c11776942e4e2a07aabbafdd Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Wed, 26 Jun 2024 22:37:02 +0530 Subject: [PATCH 22/41] Added Factory Test File Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 100 +++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 internal/metrics/otelmetrics/factory_test.go diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go new file mode 100644 index 00000000000..c63e57d4e0d --- /dev/null +++ b/internal/metrics/otelmetrics/factory_test.go @@ -0,0 +1,100 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package otelmetrics_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/exporters/prometheus" + "go.opentelemetry.io/otel/metric/noop" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + + "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" + "github.com/jaegertracing/jaeger/pkg/metrics" +) + +func newTestFactory() metrics.Factory { + exporter, _ := prometheus.New() + meterProvider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(exporter)) + return otelmetrics.NewFactory(meterProvider) +} + +func TestCounter(t *testing.T) { + factory := newTestFactory() + counter := factory.Counter(metrics.Options{ + Name: "test_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, counter) + counter.Inc(1) +} + +func TestGauge(t *testing.T) { + factory := newTestFactory() + gauge := factory.Gauge(metrics.Options{ + Name: "test_gauge", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, gauge) + gauge.Update(1) +} + +func TestHistogram(t *testing.T) { + factory := newTestFactory() + histogram := factory.Histogram(metrics.HistogramOptions{ + Name: "test_histogram", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, histogram) + histogram.Record(1.0) +} + +func TestTimer(t *testing.T) { + factory := newTestFactory() + timer := factory.Timer(metrics.TimerOptions{ + Name: "test_timer", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, timer) + timer.Record(100 * time.Millisecond) +} + +func TestNamespace(t *testing.T) { + factory := newTestFactory() + nsFactory := factory.Namespace(metrics.NSOptions{ + Name: "namespace", + Tags: map[string]string{"ns_tag1": "ns_value1"}, + }) + + counter := nsFactory.Counter(metrics.Options{ + Name: "test_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, counter) + counter.Inc(1) +} + +func TestNormalization(t *testing.T) { + factory := newTestFactory() + normalizedFactory := factory.Namespace(metrics.NSOptions{ + Name: "My Namespace", + }) + + gauge := normalizedFactory.Gauge(metrics.Options{ + Name: "My Gauge", + }) + require.NotNil(t, gauge) +} + +func TestNoopMeterProvider(t *testing.T) { + noOpFactory := otelmetrics.NewFactory(noop.NewMeterProvider()) + counter := noOpFactory.Counter(metrics.Options{ + Name: "noop_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, counter) + counter.Inc(1) +} From f6fce72705fa31eebb680d624ec54bf62b76012d Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Wed, 26 Jun 2024 22:45:32 +0530 Subject: [PATCH 23/41] Added missing value Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index c63e57d4e0d..e2105e8368c 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -16,8 +16,9 @@ import ( "github.com/jaegertracing/jaeger/pkg/metrics" ) -func newTestFactory() metrics.Factory { - exporter, _ := prometheus.New() +func newTestFactory(t *testing.T) metrics.Factory { + exporter, err := prometheus.New() + require.NoError(t, err) meterProvider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(exporter)) return otelmetrics.NewFactory(meterProvider) } From a42e1feb2cd88a1ef73409ea3e33d7477081ac24 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Wed, 26 Jun 2024 22:46:21 +0530 Subject: [PATCH 24/41] Added missing value Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index e2105e8368c..2e866854778 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -24,7 +24,7 @@ func newTestFactory(t *testing.T) metrics.Factory { } func TestCounter(t *testing.T) { - factory := newTestFactory() + factory := newTestFactory(t) counter := factory.Counter(metrics.Options{ Name: "test_counter", Tags: map[string]string{"tag1": "value1"}, @@ -34,7 +34,7 @@ func TestCounter(t *testing.T) { } func TestGauge(t *testing.T) { - factory := newTestFactory() + factory := newTestFactory(t) gauge := factory.Gauge(metrics.Options{ Name: "test_gauge", Tags: map[string]string{"tag1": "value1"}, @@ -44,7 +44,7 @@ func TestGauge(t *testing.T) { } func TestHistogram(t *testing.T) { - factory := newTestFactory() + factory := newTestFactory(t) histogram := factory.Histogram(metrics.HistogramOptions{ Name: "test_histogram", Tags: map[string]string{"tag1": "value1"}, @@ -54,7 +54,7 @@ func TestHistogram(t *testing.T) { } func TestTimer(t *testing.T) { - factory := newTestFactory() + factory := newTestFactory(t) timer := factory.Timer(metrics.TimerOptions{ Name: "test_timer", Tags: map[string]string{"tag1": "value1"}, @@ -64,7 +64,7 @@ func TestTimer(t *testing.T) { } func TestNamespace(t *testing.T) { - factory := newTestFactory() + factory := newTestFactory(t) nsFactory := factory.Namespace(metrics.NSOptions{ Name: "namespace", Tags: map[string]string{"ns_tag1": "ns_value1"}, @@ -79,7 +79,7 @@ func TestNamespace(t *testing.T) { } func TestNormalization(t *testing.T) { - factory := newTestFactory() + factory := newTestFactory(t) normalizedFactory := factory.Namespace(metrics.NSOptions{ Name: "My Namespace", }) From 3594d802302d2563c5cf4748bee00478c30622ae Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Thu, 27 Jun 2024 15:25:44 +0530 Subject: [PATCH 25/41] Made changes to Test Suites Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 65 +++++++++++++++++--- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 2e866854778..b89c28093d6 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -7,6 +7,9 @@ import ( "testing" "time" + promReg "github.com/prometheus/client_golang/prometheus" + promModel "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/metric/noop" @@ -16,55 +19,98 @@ import ( "github.com/jaegertracing/jaeger/pkg/metrics" ) -func newTestFactory(t *testing.T) metrics.Factory { - exporter, err := prometheus.New() +func newTestFactory(t *testing.T, registry *promReg.Registry) metrics.Factory { + exporter, err := prometheus.New(prometheus.WithRegisterer(registry)) require.NoError(t, err) meterProvider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(exporter)) return otelmetrics.NewFactory(meterProvider) } +func findMetric(t *testing.T, registry *promReg.Registry, name string) *promModel.MetricFamily { + metricFamilies, err := registry.Gather() + require.NoError(t, err) + + for _, mf := range metricFamilies { + t.Log(mf.GetName()) + if mf.GetName() == name { + return mf + } + } + return nil +} + func TestCounter(t *testing.T) { - factory := newTestFactory(t) + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) counter := factory.Counter(metrics.Options{ Name: "test_counter", Tags: map[string]string{"tag1": "value1"}, }) require.NotNil(t, counter) counter.Inc(1) + counter.Inc(1) + + // Find the metric in the registry + testCounter := findMetric(t, registry, "test_counter_total") + require.NotNil(t, testCounter, "Expected to find test_counter_total metric family") + + // Check the metric values + metrics := testCounter.GetMetric() + assert.Equal(t, float64(2), metrics[0].GetCounter().GetValue()) } func TestGauge(t *testing.T) { - factory := newTestFactory(t) + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) gauge := factory.Gauge(metrics.Options{ Name: "test_gauge", Tags: map[string]string{"tag1": "value1"}, }) require.NotNil(t, gauge) - gauge.Update(1) + gauge.Update(2) + + testGauge := findMetric(t, registry, "test_gauge") + require.NotNil(t, testGauge, "Expected to find test_gauge metric family") + + metrics := testGauge.GetMetric() + assert.Equal(t, float64(2), metrics[0].GetGauge().GetValue()) } func TestHistogram(t *testing.T) { - factory := newTestFactory(t) + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) histogram := factory.Histogram(metrics.HistogramOptions{ Name: "test_histogram", Tags: map[string]string{"tag1": "value1"}, }) require.NotNil(t, histogram) histogram.Record(1.0) + + testHistogram := findMetric(t, registry, "test_histogram") + require.NotNil(t, testHistogram, "Expected to find test_histogram metric family") + metrics := testHistogram.GetMetric() + assert.Equal(t, float64(1), metrics[0].GetHistogram().GetSampleSum()) } func TestTimer(t *testing.T) { - factory := newTestFactory(t) + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) timer := factory.Timer(metrics.TimerOptions{ Name: "test_timer", Tags: map[string]string{"tag1": "value1"}, }) require.NotNil(t, timer) timer.Record(100 * time.Millisecond) + + testTimer := findMetric(t, registry, "test_timer_seconds") + require.NotNil(t, testTimer, "Expected to find test_timer metric family") + metrics := testTimer.GetMetric() + assert.Equal(t, float64(0.1), metrics[0].GetHistogram().GetSampleSum()) } func TestNamespace(t *testing.T) { - factory := newTestFactory(t) + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) nsFactory := factory.Namespace(metrics.NSOptions{ Name: "namespace", Tags: map[string]string{"ns_tag1": "ns_value1"}, @@ -79,7 +125,8 @@ func TestNamespace(t *testing.T) { } func TestNormalization(t *testing.T) { - factory := newTestFactory(t) + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) normalizedFactory := factory.Namespace(metrics.NSOptions{ Name: "My Namespace", }) From 0a24e8741f5f1402167e26cb66593676b69e88a9 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Thu, 27 Jun 2024 20:00:41 +0530 Subject: [PATCH 26/41] Made Required changes in Test Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index b89c28093d6..9f160ca49e6 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -36,6 +36,7 @@ func findMetric(t *testing.T, registry *promReg.Registry, name string) *promMode return mf } } + require.NotNil(t, name, "Expected to find Metric Family") return nil } @@ -52,7 +53,6 @@ func TestCounter(t *testing.T) { // Find the metric in the registry testCounter := findMetric(t, registry, "test_counter_total") - require.NotNil(t, testCounter, "Expected to find test_counter_total metric family") // Check the metric values metrics := testCounter.GetMetric() @@ -70,7 +70,6 @@ func TestGauge(t *testing.T) { gauge.Update(2) testGauge := findMetric(t, registry, "test_gauge") - require.NotNil(t, testGauge, "Expected to find test_gauge metric family") metrics := testGauge.GetMetric() assert.Equal(t, float64(2), metrics[0].GetGauge().GetValue()) @@ -87,7 +86,7 @@ func TestHistogram(t *testing.T) { histogram.Record(1.0) testHistogram := findMetric(t, registry, "test_histogram") - require.NotNil(t, testHistogram, "Expected to find test_histogram metric family") + metrics := testHistogram.GetMetric() assert.Equal(t, float64(1), metrics[0].GetHistogram().GetSampleSum()) } @@ -103,7 +102,7 @@ func TestTimer(t *testing.T) { timer.Record(100 * time.Millisecond) testTimer := findMetric(t, registry, "test_timer_seconds") - require.NotNil(t, testTimer, "Expected to find test_timer metric family") + metrics := testTimer.GetMetric() assert.Equal(t, float64(0.1), metrics[0].GetHistogram().GetSampleSum()) } @@ -122,6 +121,12 @@ func TestNamespace(t *testing.T) { }) require.NotNil(t, counter) counter.Inc(1) + + + testCounter := findMetric(t, registry, "namespace_test_counter_total") + + metrics := testCounter.GetMetric() + assert.Equal(t, float64(1), metrics[0].GetCounter().GetValue()) } func TestNormalization(t *testing.T) { @@ -135,6 +140,12 @@ func TestNormalization(t *testing.T) { Name: "My Gauge", }) require.NotNil(t, gauge) + gauge.Update(1) + + testGauge := findMetric(t, registry, "My_Namespace_My_Gauge") + + metrics := testGauge.GetMetric() + assert.Equal(t, float64(1), metrics[0].GetGauge().GetValue()) } func TestNoopMeterProvider(t *testing.T) { From 362ea74c02ece0bbb4bac22be67eddfe3f42fe8b Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Thu, 27 Jun 2024 20:04:16 +0530 Subject: [PATCH 27/41] Removed No-Op Test Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 9f160ca49e6..34108bd0007 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/exporters/prometheus" - "go.opentelemetry.io/otel/metric/noop" sdkmetric "go.opentelemetry.io/otel/sdk/metric" "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" @@ -122,7 +121,6 @@ func TestNamespace(t *testing.T) { require.NotNil(t, counter) counter.Inc(1) - testCounter := findMetric(t, registry, "namespace_test_counter_total") metrics := testCounter.GetMetric() @@ -147,13 +145,3 @@ func TestNormalization(t *testing.T) { metrics := testGauge.GetMetric() assert.Equal(t, float64(1), metrics[0].GetGauge().GetValue()) } - -func TestNoopMeterProvider(t *testing.T) { - noOpFactory := otelmetrics.NewFactory(noop.NewMeterProvider()) - counter := noOpFactory.Counter(metrics.Options{ - Name: "noop_counter", - Tags: map[string]string{"tag1": "value1"}, - }) - require.NotNil(t, counter) - counter.Inc(1) -} From 7deeb6b8e562fb44d09eca091146efb028cb9740 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Thu, 27 Jun 2024 20:26:48 +0530 Subject: [PATCH 28/41] Added Go Leak Test Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 5 +++++ internal/metrics/otelmetrics/factory_test.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index 73d133adbe0..7e5935ce8af 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -14,8 +14,13 @@ import ( "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" prom "github.com/jaegertracing/jaeger/internal/metrics/prometheus" "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/testutils" ) +func TestMain(m *testing.M) { + testutils.VerifyGoLeaks(m) +} + func setupPrometheusFactory() metrics.Factory { reg := prometheus.NewRegistry() return prom.New(prom.WithRegisterer(reg)) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 34108bd0007..5063a939c11 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -16,8 +16,13 @@ import ( "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/testutils" ) +func TestMain(m *testing.M) { + testutils.VerifyGoLeaks(m) +} + func newTestFactory(t *testing.T, registry *promReg.Registry) metrics.Factory { exporter, err := prometheus.New(prometheus.WithRegisterer(registry)) require.NoError(t, err) From 92ea1bb2317c9185db7ab678e7110d00d78bf22d Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Thu, 27 Jun 2024 20:39:31 +0530 Subject: [PATCH 29/41] Fixed Alias Error Signed-off-by: Wise-Wizard --- internal/metrics/benchmark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/benchmark_test.go b/internal/metrics/benchmark_test.go index 7e5935ce8af..62c322c0b91 100644 --- a/internal/metrics/benchmark_test.go +++ b/internal/metrics/benchmark_test.go @@ -6,7 +6,7 @@ package benchmark_test import ( "testing" - prometheus "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" promExporter "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" From 47c2aed704d195112b37f7c9f8959a29e7efd6f8 Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Thu, 27 Jun 2024 21:03:48 +0530 Subject: [PATCH 30/41] Update internal/metrics/otelmetrics/factory_test.go Co-authored-by: Yuri Shkuro Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> --- internal/metrics/otelmetrics/factory_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 5063a939c11..9c3657810cf 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -55,10 +55,7 @@ func TestCounter(t *testing.T) { counter.Inc(1) counter.Inc(1) - // Find the metric in the registry testCounter := findMetric(t, registry, "test_counter_total") - - // Check the metric values metrics := testCounter.GetMetric() assert.Equal(t, float64(2), metrics[0].GetCounter().GetValue()) } From 36dda89f7d5a3b6d2d32a6b6093ec95d85682532 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Thu, 27 Jun 2024 22:50:18 +0530 Subject: [PATCH 31/41] Implemented missing Tests Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 36 +++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 9c3657810cf..0b42bba9cb6 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -40,10 +40,34 @@ func findMetric(t *testing.T, registry *promReg.Registry, name string) *promMode return mf } } - require.NotNil(t, name, "Expected to find Metric Family") + require.Fail(t, "Expected to find Metric Family") return nil } +func checkLabels(t *testing.T, expectedLabels map[string]string, labels []*promModel.LabelPair) { + for key, value := range expectedLabels { + found := false + for _, label := range labels { + if label.GetName() == key { + assert.Equal(t, value, label.GetValue()) + found = true + break + } + } + assert.True(t, found, "Label %s not found", key) + } +} + +func TestInvalidMetric(t *testing.T) { + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) + counter := factory.Counter(metrics.Options{ + Name: "invalid*name%", + Tags: map[string]string{"tag1": "value1"}, + }) + assert.True(t, counter == metrics.NullCounter, "Expected NullCounter, got %v", counter) +} + func TestCounter(t *testing.T) { registry := promReg.NewPedanticRegistry() factory := newTestFactory(t, registry) @@ -58,6 +82,8 @@ func TestCounter(t *testing.T) { testCounter := findMetric(t, registry, "test_counter_total") metrics := testCounter.GetMetric() assert.Equal(t, float64(2), metrics[0].GetCounter().GetValue()) + expectedLabels := map[string]string{"tag1": "value1"} + checkLabels(t, expectedLabels, metrics[0].GetLabel()) } func TestGauge(t *testing.T) { @@ -74,6 +100,8 @@ func TestGauge(t *testing.T) { metrics := testGauge.GetMetric() assert.Equal(t, float64(2), metrics[0].GetGauge().GetValue()) + expectedLabels := map[string]string{"tag1": "value1"} + checkLabels(t, expectedLabels, metrics[0].GetLabel()) } func TestHistogram(t *testing.T) { @@ -90,6 +118,8 @@ func TestHistogram(t *testing.T) { metrics := testHistogram.GetMetric() assert.Equal(t, float64(1), metrics[0].GetHistogram().GetSampleSum()) + expectedLabels := map[string]string{"tag1": "value1"} + checkLabels(t, expectedLabels, metrics[0].GetLabel()) } func TestTimer(t *testing.T) { @@ -106,6 +136,8 @@ func TestTimer(t *testing.T) { metrics := testTimer.GetMetric() assert.Equal(t, float64(0.1), metrics[0].GetHistogram().GetSampleSum()) + expectedLabels := map[string]string{"tag1": "value1"} + checkLabels(t, expectedLabels, metrics[0].GetLabel()) } func TestNamespace(t *testing.T) { @@ -127,6 +159,8 @@ func TestNamespace(t *testing.T) { metrics := testCounter.GetMetric() assert.Equal(t, float64(1), metrics[0].GetCounter().GetValue()) + expectedLabels := map[string]string{"tag1": "value1"} + checkLabels(t, expectedLabels, metrics[0].GetLabel()) } func TestNormalization(t *testing.T) { From d4ee6e0a2a2d0675f3d0b9e53252a885c2325f09 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 28 Jun 2024 02:04:02 +0530 Subject: [PATCH 32/41] Created Table Driven Test Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 135 +++++++++++++------ 1 file changed, 95 insertions(+), 40 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 0b42bba9cb6..255cbe6030f 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -35,7 +35,6 @@ func findMetric(t *testing.T, registry *promReg.Registry, name string) *promMode require.NoError(t, err) for _, mf := range metricFamilies { - t.Log(mf.GetName()) if mf.GetName() == name { return mf } @@ -44,18 +43,12 @@ func findMetric(t *testing.T, registry *promReg.Registry, name string) *promMode return nil } -func checkLabels(t *testing.T, expectedLabels map[string]string, labels []*promModel.LabelPair) { - for key, value := range expectedLabels { - found := false - for _, label := range labels { - if label.GetName() == key { - assert.Equal(t, value, label.GetValue()) - found = true - break - } - } - assert.True(t, found, "Label %s not found", key) +func promLabelsToMap(labels []*promModel.LabelPair) map[string]string { + labelMap := make(map[string]string) + for _, label := range labels { + labelMap[label.GetName()] = label.GetValue() } + return labelMap } func TestInvalidMetric(t *testing.T) { @@ -65,7 +58,7 @@ func TestInvalidMetric(t *testing.T) { Name: "invalid*name%", Tags: map[string]string{"tag1": "value1"}, }) - assert.True(t, counter == metrics.NullCounter, "Expected NullCounter, got %v", counter) + assert.Equal(t, counter, metrics.NullCounter, "Expected NullCounter, got %v", counter) } func TestCounter(t *testing.T) { @@ -82,8 +75,12 @@ func TestCounter(t *testing.T) { testCounter := findMetric(t, registry, "test_counter_total") metrics := testCounter.GetMetric() assert.Equal(t, float64(2), metrics[0].GetCounter().GetValue()) - expectedLabels := map[string]string{"tag1": "value1"} - checkLabels(t, expectedLabels, metrics[0].GetLabel()) + expectedLabels := map[string]string{ + "tag1": "value1", + "otel_scope_name": "jaeger-v2", + "otel_scope_version": "", + } + assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } func TestGauge(t *testing.T) { @@ -100,8 +97,12 @@ func TestGauge(t *testing.T) { metrics := testGauge.GetMetric() assert.Equal(t, float64(2), metrics[0].GetGauge().GetValue()) - expectedLabels := map[string]string{"tag1": "value1"} - checkLabels(t, expectedLabels, metrics[0].GetLabel()) + expectedLabels := map[string]string{ + "tag1": "value1", + "otel_scope_name": "jaeger-v2", + "otel_scope_version": "", + } + assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } func TestHistogram(t *testing.T) { @@ -118,8 +119,12 @@ func TestHistogram(t *testing.T) { metrics := testHistogram.GetMetric() assert.Equal(t, float64(1), metrics[0].GetHistogram().GetSampleSum()) - expectedLabels := map[string]string{"tag1": "value1"} - checkLabels(t, expectedLabels, metrics[0].GetLabel()) + expectedLabels := map[string]string{ + "tag1": "value1", + "otel_scope_name": "jaeger-v2", + "otel_scope_version": "", + } + assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } func TestTimer(t *testing.T) { @@ -136,31 +141,81 @@ func TestTimer(t *testing.T) { metrics := testTimer.GetMetric() assert.Equal(t, float64(0.1), metrics[0].GetHistogram().GetSampleSum()) - expectedLabels := map[string]string{"tag1": "value1"} - checkLabels(t, expectedLabels, metrics[0].GetLabel()) + expectedLabels := map[string]string{ + "tag1": "value1", + "otel_scope_name": "jaeger-v2", + "otel_scope_version": "", + } + assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } func TestNamespace(t *testing.T) { - registry := promReg.NewPedanticRegistry() - factory := newTestFactory(t, registry) - nsFactory := factory.Namespace(metrics.NSOptions{ - Name: "namespace", - Tags: map[string]string{"ns_tag1": "ns_value1"}, - }) - - counter := nsFactory.Counter(metrics.Options{ - Name: "test_counter", - Tags: map[string]string{"tag1": "value1"}, - }) - require.NotNil(t, counter) - counter.Inc(1) - - testCounter := findMetric(t, registry, "namespace_test_counter_total") + testCases := []struct { + nsOptions metrics.NSOptions + expectedName string + expectedLabels map[string]string + }{ + { + nsOptions: metrics.NSOptions{ + Name: "first_namespace", + Tags: map[string]string{"ns_tag1": "ns_value1"}, + }, + expectedName: "first_namespace_test_counter_total", + expectedLabels: map[string]string{ + "ns_tag1": "ns_value1", + "tag1": "value1", + "otel_scope_name": "jaeger-v2", + "otel_scope_version": "", + }, + }, + { + nsOptions: metrics.NSOptions{ + Name: "second_namespace", + Tags: map[string]string{"ns_tag2": "ns_value2"}, + }, + expectedName: "second_namespace_test_counter_total", + expectedLabels: map[string]string{ + "ns_tag2": "ns_value2", + "tag1": "value1", + "otel_scope_name": "jaeger-v2", + "otel_scope_version": "", + }, + }, + { + nsOptions: metrics.NSOptions{ + Name: "third_namespace", + Tags: map[string]string{"ns_tag3": "ns_value3"}, + }, + expectedName: "third_namespace_test_counter_total", + expectedLabels: map[string]string{ + "ns_tag3": "ns_value3", + "tag1": "value1", + "otel_scope_name": "jaeger-v2", + "otel_scope_version": "", + }, + }, + } - metrics := testCounter.GetMetric() - assert.Equal(t, float64(1), metrics[0].GetCounter().GetValue()) - expectedLabels := map[string]string{"tag1": "value1"} - checkLabels(t, expectedLabels, metrics[0].GetLabel()) + for _, tc := range testCases { + t.Run(tc.expectedName, func(t *testing.T) { + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) + nsFactory := factory.Namespace(tc.nsOptions) + + counter := nsFactory.Counter(metrics.Options{ + Name: "test_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, counter) + counter.Inc(1) + + testCounter := findMetric(t, registry, tc.expectedName) + + metrics := testCounter.GetMetric() + assert.Equal(t, float64(1), metrics[0].GetCounter().GetValue()) + assert.Equal(t, tc.expectedLabels, promLabelsToMap(metrics[0].GetLabel())) + }) + } } func TestNormalization(t *testing.T) { From 5e7c6241e680ad411a655265a4efa5f6ed0bab9e Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 28 Jun 2024 10:19:36 +0530 Subject: [PATCH 33/41] Added invalid test for all metrics Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 32 ++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 255cbe6030f..3dc00a11698 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -51,16 +51,42 @@ func promLabelsToMap(labels []*promModel.LabelPair) map[string]string { return labelMap } -func TestInvalidMetric(t *testing.T) { +func TestInvalidCounter(t *testing.T) { registry := promReg.NewPedanticRegistry() factory := newTestFactory(t, registry) counter := factory.Counter(metrics.Options{ - Name: "invalid*name%", - Tags: map[string]string{"tag1": "value1"}, + Name: "invalid*counter%", }) assert.Equal(t, counter, metrics.NullCounter, "Expected NullCounter, got %v", counter) } +func TestInvalidGauge(t *testing.T) { + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) + gauge := factory.Gauge(metrics.Options{ + Name: "#invalid>gauge%", + }) + assert.Equal(t, gauge, metrics.NullGauge, "Expected NullCounter, got %v", gauge) +} + +func TestInvalidHistogram(t *testing.T) { + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) + histogram := factory.Histogram(metrics.HistogramOptions{ + Name: "invalid>histogram?%", + }) + assert.Equal(t, histogram, metrics.NullHistogram, "Expected NullCounter, got %v", histogram) +} + +func TestInvalidTimer(t *testing.T) { + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) + timer := factory.Timer(metrics.TimerOptions{ + Name: "invalid*<=timer%", + }) + assert.Equal(t, timer, metrics.NullTimer, "Expected NullCounter, got %v", timer) +} + func TestCounter(t *testing.T) { registry := promReg.NewPedanticRegistry() factory := newTestFactory(t, registry) From 965ad8495a453655d8f08eb6c38d19dc958f1479 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 28 Jun 2024 11:15:21 +0530 Subject: [PATCH 34/41] Added check for Empty Namespace Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 3dc00a11698..f7682b92c99 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -196,10 +196,10 @@ func TestNamespace(t *testing.T) { }, { nsOptions: metrics.NSOptions{ - Name: "second_namespace", + Name: "", Tags: map[string]string{"ns_tag2": "ns_value2"}, }, - expectedName: "second_namespace_test_counter_total", + expectedName: "test_counter_total", expectedLabels: map[string]string{ "ns_tag2": "ns_value2", "tag1": "value1", @@ -207,19 +207,6 @@ func TestNamespace(t *testing.T) { "otel_scope_version": "", }, }, - { - nsOptions: metrics.NSOptions{ - Name: "third_namespace", - Tags: map[string]string{"ns_tag3": "ns_value3"}, - }, - expectedName: "third_namespace_test_counter_total", - expectedLabels: map[string]string{ - "ns_tag3": "ns_value3", - "tag1": "value1", - "otel_scope_name": "jaeger-v2", - "otel_scope_version": "", - }, - }, } for _, tc := range testCases { From 43de5d1428bd2ed8ee738db601967dc649da1a76 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 28 Jun 2024 11:26:15 +0530 Subject: [PATCH 35/41] Disabled OTEL Scope Info Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 22 +++++--------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index f7682b92c99..cbccab3f03f 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -24,7 +24,7 @@ func TestMain(m *testing.M) { } func newTestFactory(t *testing.T, registry *promReg.Registry) metrics.Factory { - exporter, err := prometheus.New(prometheus.WithRegisterer(registry)) + exporter, err := prometheus.New(prometheus.WithRegisterer(registry), prometheus.WithoutScopeInfo()) require.NoError(t, err) meterProvider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(exporter)) return otelmetrics.NewFactory(meterProvider) @@ -102,9 +102,7 @@ func TestCounter(t *testing.T) { metrics := testCounter.GetMetric() assert.Equal(t, float64(2), metrics[0].GetCounter().GetValue()) expectedLabels := map[string]string{ - "tag1": "value1", - "otel_scope_name": "jaeger-v2", - "otel_scope_version": "", + "tag1": "value1", } assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } @@ -124,9 +122,7 @@ func TestGauge(t *testing.T) { metrics := testGauge.GetMetric() assert.Equal(t, float64(2), metrics[0].GetGauge().GetValue()) expectedLabels := map[string]string{ - "tag1": "value1", - "otel_scope_name": "jaeger-v2", - "otel_scope_version": "", + "tag1": "value1", } assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } @@ -146,9 +142,7 @@ func TestHistogram(t *testing.T) { metrics := testHistogram.GetMetric() assert.Equal(t, float64(1), metrics[0].GetHistogram().GetSampleSum()) expectedLabels := map[string]string{ - "tag1": "value1", - "otel_scope_name": "jaeger-v2", - "otel_scope_version": "", + "tag1": "value1", } assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } @@ -168,9 +162,7 @@ func TestTimer(t *testing.T) { metrics := testTimer.GetMetric() assert.Equal(t, float64(0.1), metrics[0].GetHistogram().GetSampleSum()) expectedLabels := map[string]string{ - "tag1": "value1", - "otel_scope_name": "jaeger-v2", - "otel_scope_version": "", + "tag1": "value1", } assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } @@ -190,8 +182,6 @@ func TestNamespace(t *testing.T) { expectedLabels: map[string]string{ "ns_tag1": "ns_value1", "tag1": "value1", - "otel_scope_name": "jaeger-v2", - "otel_scope_version": "", }, }, { @@ -203,8 +193,6 @@ func TestNamespace(t *testing.T) { expectedLabels: map[string]string{ "ns_tag2": "ns_value2", "tag1": "value1", - "otel_scope_name": "jaeger-v2", - "otel_scope_version": "", }, }, } From 878e5f4db86560b83c168f07add6e19dab05c434 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 28 Jun 2024 11:39:34 +0530 Subject: [PATCH 36/41] Test Nested Namespace Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 25 +++++++++++++------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index cbccab3f03f..c4b1622155f 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -35,6 +35,7 @@ func findMetric(t *testing.T, registry *promReg.Registry, name string) *promMode require.NoError(t, err) for _, mf := range metricFamilies { + t.Log(mf.GetName()) if mf.GetName() == name { return mf } @@ -173,26 +174,30 @@ func TestNamespace(t *testing.T) { expectedName string expectedLabels map[string]string }{ + //Test Nested Namespace { nsOptions: metrics.NSOptions{ Name: "first_namespace", Tags: map[string]string{"ns_tag1": "ns_value1"}, }, - expectedName: "first_namespace_test_counter_total", + expectedName: "first_namespace_second_namespace_test_counter_total", expectedLabels: map[string]string{ - "ns_tag1": "ns_value1", - "tag1": "value1", + "ns_tag1": "ns_value1", + "ns_tag3": "ns_value3", + "tag1": "value1", }, }, + //Test Empty Namespace { nsOptions: metrics.NSOptions{ Name: "", Tags: map[string]string{"ns_tag2": "ns_value2"}, }, - expectedName: "test_counter_total", + expectedName: "second_namespace_test_counter_total", expectedLabels: map[string]string{ - "ns_tag2": "ns_value2", - "tag1": "value1", + "ns_tag2": "ns_value2", + "ns_tag3": "ns_value3", + "tag1": "value1", }, }, } @@ -201,9 +206,13 @@ func TestNamespace(t *testing.T) { t.Run(tc.expectedName, func(t *testing.T) { registry := promReg.NewPedanticRegistry() factory := newTestFactory(t, registry) - nsFactory := factory.Namespace(tc.nsOptions) + nsFactory1 := factory.Namespace(tc.nsOptions) + nsFactory2 := nsFactory1.Namespace(metrics.NSOptions{ + Name: "second_namespace", + Tags: map[string]string{"ns_tag3": "ns_value3"}, + }) - counter := nsFactory.Counter(metrics.Options{ + counter := nsFactory2.Counter(metrics.Options{ Name: "test_counter", Tags: map[string]string{"tag1": "value1"}, }) From 864acde10cb17db057d9fd4e713e68bac8e31c18 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 28 Jun 2024 11:42:01 +0530 Subject: [PATCH 37/41] Ran make Lint Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index c4b1622155f..d5897b9c7b6 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -174,7 +174,7 @@ func TestNamespace(t *testing.T) { expectedName string expectedLabels map[string]string }{ - //Test Nested Namespace + // Test Nested Namespace { nsOptions: metrics.NSOptions{ Name: "first_namespace", @@ -187,15 +187,11 @@ func TestNamespace(t *testing.T) { "tag1": "value1", }, }, - //Test Empty Namespace + // Test Empty Namespace { - nsOptions: metrics.NSOptions{ - Name: "", - Tags: map[string]string{"ns_tag2": "ns_value2"}, - }, + nsOptions: metrics.NSOptions{}, expectedName: "second_namespace_test_counter_total", expectedLabels: map[string]string{ - "ns_tag2": "ns_value2", "ns_tag3": "ns_value3", "tag1": "value1", }, From 0054c5cd4f2e0627df89bc9ab1f10bb0cd037503 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Fri, 28 Jun 2024 11:52:32 +0530 Subject: [PATCH 38/41] Ran make fmt Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index d5897b9c7b6..483c04186b7 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -189,7 +189,7 @@ func TestNamespace(t *testing.T) { }, // Test Empty Namespace { - nsOptions: metrics.NSOptions{}, + nsOptions: metrics.NSOptions{}, expectedName: "second_namespace_test_counter_total", expectedLabels: map[string]string{ "ns_tag3": "ns_value3", From 7188cc8a4539150e3da5467fc514ec4a34d7aa62 Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Sun, 30 Jun 2024 14:45:43 +0530 Subject: [PATCH 39/41] Update internal/metrics/otelmetrics/factory_test.go Co-authored-by: Yuri Shkuro Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> --- internal/metrics/otelmetrics/factory_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index 483c04186b7..fe0ed031cc3 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -53,8 +53,7 @@ func promLabelsToMap(labels []*promModel.LabelPair) map[string]string { } func TestInvalidCounter(t *testing.T) { - registry := promReg.NewPedanticRegistry() - factory := newTestFactory(t, registry) + factory, registry := newTestFactory(t) counter := factory.Counter(metrics.Options{ Name: "invalid*counter%", }) From e0d7fdb1f6c4583cbfb7a7ee2faa75313478aaf6 Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Sun, 30 Jun 2024 15:07:14 +0530 Subject: [PATCH 40/41] Added Empty Namespace Test Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 33 +++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index fe0ed031cc3..f9288a6810f 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -53,7 +53,7 @@ func promLabelsToMap(labels []*promModel.LabelPair) map[string]string { } func TestInvalidCounter(t *testing.T) { - factory, registry := newTestFactory(t) + factory := newTestFactory(t, promReg.NewPedanticRegistry()) counter := factory.Counter(metrics.Options{ Name: "invalid*counter%", }) @@ -61,8 +61,7 @@ func TestInvalidCounter(t *testing.T) { } func TestInvalidGauge(t *testing.T) { - registry := promReg.NewPedanticRegistry() - factory := newTestFactory(t, registry) + factory := newTestFactory(t, promReg.NewPedanticRegistry()) gauge := factory.Gauge(metrics.Options{ Name: "#invalid>gauge%", }) @@ -70,8 +69,7 @@ func TestInvalidGauge(t *testing.T) { } func TestInvalidHistogram(t *testing.T) { - registry := promReg.NewPedanticRegistry() - factory := newTestFactory(t, registry) + factory := newTestFactory(t, promReg.NewPedanticRegistry()) histogram := factory.Histogram(metrics.HistogramOptions{ Name: "invalid>histogram?%", }) @@ -79,8 +77,7 @@ func TestInvalidHistogram(t *testing.T) { } func TestInvalidTimer(t *testing.T) { - registry := promReg.NewPedanticRegistry() - factory := newTestFactory(t, registry) + factory := newTestFactory(t, promReg.NewPedanticRegistry()) timer := factory.Timer(metrics.TimerOptions{ Name: "invalid*<=timer%", }) @@ -167,6 +164,27 @@ func TestTimer(t *testing.T) { assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } +func TestEmptyNamespace(t *testing.T) { + registry := promReg.NewPedanticRegistry() + factory := newTestFactory(t, registry) + emptyFactory := factory.Namespace(metrics.NSOptions{}) + counter := emptyFactory.Counter(metrics.Options{ + Name: "test_counter", + Tags: map[string]string{"tag1": "value1"}, + }) + require.NotNil(t, counter) + counter.Inc(1) + + testCounter := findMetric(t, registry, "test_counter_total") + + metrics := testCounter.GetMetric() + assert.Equal(t, float64(1), metrics[0].GetCounter().GetValue()) + expectedLabels := map[string]string{ + "tag1": "value1", + } + assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) +} + func TestNamespace(t *testing.T) { testCases := []struct { nsOptions metrics.NSOptions @@ -206,7 +224,6 @@ func TestNamespace(t *testing.T) { Name: "second_namespace", Tags: map[string]string{"ns_tag3": "ns_value3"}, }) - counter := nsFactory2.Counter(metrics.Options{ Name: "test_counter", Tags: map[string]string{"tag1": "value1"}, From 3265bc02cb5d6e1667754accfafe663cc43fd88c Mon Sep 17 00:00:00 2001 From: Wise-Wizard Date: Sun, 30 Jun 2024 15:35:51 +0530 Subject: [PATCH 41/41] Refactored Test Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory_test.go | 64 +++++++++----------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index f9288a6810f..ade7cbf8064 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -164,39 +164,24 @@ func TestTimer(t *testing.T) { assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } -func TestEmptyNamespace(t *testing.T) { - registry := promReg.NewPedanticRegistry() - factory := newTestFactory(t, registry) - emptyFactory := factory.Namespace(metrics.NSOptions{}) - counter := emptyFactory.Counter(metrics.Options{ - Name: "test_counter", - Tags: map[string]string{"tag1": "value1"}, - }) - require.NotNil(t, counter) - counter.Inc(1) - - testCounter := findMetric(t, registry, "test_counter_total") - - metrics := testCounter.GetMetric() - assert.Equal(t, float64(1), metrics[0].GetCounter().GetValue()) - expectedLabels := map[string]string{ - "tag1": "value1", - } - assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) -} - func TestNamespace(t *testing.T) { testCases := []struct { - nsOptions metrics.NSOptions + name string + nsOptions1 metrics.NSOptions + nsOptions2 metrics.NSOptions expectedName string expectedLabels map[string]string }{ - // Test Nested Namespace { - nsOptions: metrics.NSOptions{ + name: "Nested Namespace", + nsOptions1: metrics.NSOptions{ Name: "first_namespace", Tags: map[string]string{"ns_tag1": "ns_value1"}, }, + nsOptions2: metrics.NSOptions{ + Name: "second_namespace", + Tags: map[string]string{"ns_tag3": "ns_value3"}, + }, expectedName: "first_namespace_second_namespace_test_counter_total", expectedLabels: map[string]string{ "ns_tag1": "ns_value1", @@ -204,26 +189,37 @@ func TestNamespace(t *testing.T) { "tag1": "value1", }, }, - // Test Empty Namespace { - nsOptions: metrics.NSOptions{}, - expectedName: "second_namespace_test_counter_total", + name: "Single Namespace", + nsOptions1: metrics.NSOptions{ + Name: "single_namespace", + Tags: map[string]string{"ns_tag2": "ns_value2"}, + }, + nsOptions2: metrics.NSOptions{}, + expectedName: "single_namespace_test_counter_total", expectedLabels: map[string]string{ - "ns_tag3": "ns_value3", + "ns_tag2": "ns_value2", "tag1": "value1", }, }, + { + name: "Empty Namespace Name", + nsOptions1: metrics.NSOptions{}, + nsOptions2: metrics.NSOptions{}, + expectedName: "test_counter_total", + expectedLabels: map[string]string{ + "tag1": "value1", + }, + }, } for _, tc := range testCases { - t.Run(tc.expectedName, func(t *testing.T) { + t.Run(tc.name, func(t *testing.T) { registry := promReg.NewPedanticRegistry() factory := newTestFactory(t, registry) - nsFactory1 := factory.Namespace(tc.nsOptions) - nsFactory2 := nsFactory1.Namespace(metrics.NSOptions{ - Name: "second_namespace", - Tags: map[string]string{"ns_tag3": "ns_value3"}, - }) + nsFactory1 := factory.Namespace(tc.nsOptions1) + nsFactory2 := nsFactory1.Namespace(tc.nsOptions2) + counter := nsFactory2.Counter(metrics.Options{ Name: "test_counter", Tags: map[string]string{"tag1": "value1"},