From 4a676973f65c0b4e0f64c46364d39df0ede9bc0f Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Thu, 8 Aug 2024 06:35:49 +0530 Subject: [PATCH] Added _total suffix to OTEL counter metrics. (#5810) **Which problem is this PR solving?** This PR addresses a part of the issue [#5633 ](https://github.com/jaegertracing/jaeger/issues/5633) **Description of the changes** This is a PR to achieve Observability Parity in metrics between V1 and V2 components by configuring OTEL Collector to emit desired metrics. **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` ```bash CI actions and compare.py script ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md) - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - `for jaeger: make lint test` - `for jaeger-ui: yarn lint` and `yarn test` --------- Signed-off-by: Wise-Wizard Signed-off-by: Jared Tan --- internal/metrics/otelmetrics/factory.go | 9 ++++++++- internal/metrics/otelmetrics/factory_test.go | 11 +++++++++++ scripts/compare_metrics.py | 9 ++------- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index d14e60c616c..17adec85a04 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -32,7 +32,7 @@ func NewFactory(meterProvider metric.MeterProvider) metrics.Factory { } func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { - name := f.subScope(opts.Name) + name := CounterNamingConvention(f.subScope(opts.Name)) counter, err := f.meter.Int64Counter(name) if err != nil { log.Printf("Error creating OTEL counter: %v", err) @@ -131,3 +131,10 @@ func attributeSetOption(tags map[string]string) metric.MeasurementOption { } return metric.WithAttributes(attributes...) } + +func CounterNamingConvention(name string) string { + if !strings.HasSuffix(name, "_total") { + name += "_total" + } + return name +} diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index ade7cbf8064..593e4bb9ade 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -104,6 +104,17 @@ func TestCounter(t *testing.T) { assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } +func TestCounterNamingConvention(t *testing.T) { + input := "test_counter" + expected := "test_counter_total" + + result := otelmetrics.CounterNamingConvention(input) + + if result != expected { + t.Errorf("Expected %s, but got %s", expected, result) + } +} + func TestGauge(t *testing.T) { registry := promReg.NewPedanticRegistry() factory := newTestFactory(t, registry) diff --git a/scripts/compare_metrics.py b/scripts/compare_metrics.py index 8869f1826c6..a34d25556b8 100644 --- a/scripts/compare_metrics.py +++ b/scripts/compare_metrics.py @@ -1,9 +1,3 @@ -# Run the following commands first to create the JSON files: -# Run V1 Binary -# prom2json http://localhost:14269/metrics > V1_Metrics.json -# Run V2 Binary -# prom2json http://localhost:8888/metrics > V2_Metrics.json - import json v1_metrics_path = "./V1_Metrics.json" @@ -41,7 +35,7 @@ def extract_metrics_with_labels(metrics, strip_prefix=None): for name, labels in v1_metrics_with_labels.items(): if name in v2_metrics_with_labels: common_metrics[name] = labels - else: + elif not name.startswith("jaeger_agent"): v1_only_metrics[name] = labels for name, labels in v2_metrics_with_labels.items(): @@ -54,6 +48,7 @@ def extract_metrics_with_labels(metrics, strip_prefix=None): "v2_only_metrics": v2_only_metrics } +# Write the differences to a new JSON file differences_path = "./differences.json" with open(differences_path, 'w') as file: json.dump(differences, file, indent=4)