From ada556991ab8743613b4f7f4452d01718c2a02c9 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Tue, 11 Jun 2024 19:23:34 +0530 Subject: [PATCH] Remove support for OTEL spanmetrics processor (#5539) --- docker-compose/monitor/Makefile | 11 --- docker-compose/monitor/README.md | 60 +++------------- .../otel-collector-config-processor.yml | 42 ----------- pkg/prometheus/config/config.go | 9 ++- plugin/metrics/prometheus/factory_test.go | 13 ---- .../metricsstore/dbmodel/to_domain.go | 4 +- .../metricsstore/dbmodel/to_domain_test.go | 2 +- .../metrics/prometheus/metricsstore/reader.go | 15 +--- .../prometheus/metricsstore/reader_test.go | 72 ++++++++++--------- ...service_span_name_datapoint_response.json} | 2 +- plugin/metrics/prometheus/options.go | 29 +++----- 11 files changed, 66 insertions(+), 193 deletions(-) delete mode 100644 docker-compose/monitor/otel-collector-config-processor.yml rename plugin/metrics/prometheus/metricsstore/testdata/{service_operation_datapoint_response.json => service_span_name_datapoint_response.json} (88%) diff --git a/docker-compose/monitor/Makefile b/docker-compose/monitor/Makefile index 794854fdb96..caa2934fc1f 100644 --- a/docker-compose/monitor/Makefile +++ b/docker-compose/monitor/Makefile @@ -12,17 +12,6 @@ dev: export JAEGER_IMAGE_TAG = dev dev: docker compose -f docker-compose.yml up -# starts older spanmetrics processor setup, for example, -# to test backwards compatibility of Jaeger with spanmetrics processor. -.PHONY: dev-processor -dev-processor: export JAEGER_IMAGE_TAG = dev -# Fix to a version before the breaking changes were introduced. -dev-processor: export OTEL_IMAGE_TAG = 0.70.0 -dev-processor: export OTEL_CONFIG_SRC = ./otel-collector-config-processor.yml -dev-processor: export PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR = false -dev-processor: - docker compose -f docker-compose.yml up - .PHONY: clean-jaeger clean-jaeger: # Also cleans up intermediate cached containers. diff --git a/docker-compose/monitor/README.md b/docker-compose/monitor/README.md index e9181ae1ccf..311fc10ca75 100644 --- a/docker-compose/monitor/README.md +++ b/docker-compose/monitor/README.md @@ -15,7 +15,7 @@ This environment consists the following backend components: - [MicroSim](https://github.com/yurishkuro/microsim): a program to simulate traces. - [Jaeger All-in-one](https://www.jaegertracing.io/docs/1.24/getting-started/#all-in-one): the full Jaeger stack in a single container image. -- [OpenTelemetry Collector](https://opentelemetry.io/docs/collector/): vendor agnostic integration layer for traces and metrics. Its main role in this particular development environment is to receive Jaeger spans, forward these spans untouched to Jaeger All-in-one while simultaneously aggregating metrics out of this span data. To learn more about span metrics aggregation, please refer to the [spanmetrics processor documentation][spanmetricsprocessor]. +- [OpenTelemetry Collector](https://opentelemetry.io/docs/collector/): vendor agnostic integration layer for traces and metrics. Its main role in this particular development environment is to receive Jaeger spans, forward these spans untouched to Jaeger All-in-one while simultaneously aggregating metrics out of this span data. To learn more about span metrics aggregation, please refer to the [spanmetrics connector documentation][spanmetricsconnectorreadme]. - [Prometheus](https://prometheus.io/): a metrics collection and query engine, used to scrape metrics computed by OpenTelemetry Collector, and presents an API for Jaeger All-in-one to query these metrics. - [Grafana](https://grafana.com/): a metrics visualization, analytics & monitoring solution supporting multiple metrics databases. @@ -65,16 +65,6 @@ make build make dev ``` -## Backwards compatibility testing with spanmetrics processor - -```bash -make dev-processor -``` - -For each "run" make target, you should expect to see the following in the Monitor tab after a few minutes: - -![Monitor Screenshot](images/startup-monitor-tab.png) - ## Sending traces We will use [tracegen](https://github.com/jaegertracing/jaeger/tree/main/cmd/tracegen) @@ -114,45 +104,6 @@ Then navigate to the Monitor tab at http://localhost:16686/monitor to view the R ![TraceGen RED Metrics](images/tracegen_metrics.png) -## Migrating to Span Metrics Connector - -### Background - -A new [Connector](https://pkg.go.dev/go.opentelemetry.io/collector/connector#section-readme) API was introduced -to the OpenTelemetry Collector to provide a means of receiving and exporting between any type of telemetry. - -The existing [Span Metrics Processor][spanmetricsprocessor] was a good candidate to migrate over to the connector type, -resulting in the new [Span Metrics Connector][spanmetricsconnector] component. - -The Span Metrics Connector variant introduces some [breaking changes][processor-to-connector], and the following -section aims to provide the instructions necessary to use the metrics produced by this component. - -### Migrating - -Assuming the OpenTelemetry Collector is running with the [Span Metrics Connector][spanmetricsconnector] correctly -configured, the minimum configuration required for jaeger-query or jaeger-all-in-one are as follows: - -as command line parameters: -```shell ---prometheus.query.support-spanmetrics-connector=true -``` - -as environment variables: -```shell -PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR=true -``` - -If the Span Metrics Connector is configured with a namespace and/or an alternative duration unit, -the following configuration options are available, as both command line and environment variables: - -```shell ---prometheus.query.namespace=span_metrics ---prometheus.query.duration-unit=s - -PROMETHEUS_QUERY_NAMESPACE=span_metrics -PROMETHEUS_QUERY_DURATION_UNIT=s -``` - ## Querying the HTTP API ### Example 1 @@ -278,7 +229,13 @@ For example: }, "timestamp": "2021-06-03T09:12:11Z" }, + ] +... + } ... + ] +... +} ``` If the `groupByOperation=true` parameter is set, the response will include the operation name in the labels like so: @@ -318,6 +275,5 @@ $ curl http://localhost:16686/api/metrics/minstep | jq . } ``` -[spanmetricsprocessor]: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/spanmetricsprocessor [spanmetricsconnector]: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/connector/spanmetricsconnector -[processor-to-connector]: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/connector/spanmetricsconnector#span-to-metrics-processor-to-span-to-metrics-connector +[spanmetricsconnectorreadme]: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/connector/spanmetricsconnector/README.md diff --git a/docker-compose/monitor/otel-collector-config-processor.yml b/docker-compose/monitor/otel-collector-config-processor.yml deleted file mode 100644 index 5ca7d7b1b44..00000000000 --- a/docker-compose/monitor/otel-collector-config-processor.yml +++ /dev/null @@ -1,42 +0,0 @@ -receivers: - jaeger: - protocols: - thrift_http: - endpoint: "0.0.0.0:14278" - - otlp: - protocols: - grpc: - http: - - # Dummy receiver that's never used, because a pipeline is required to have one. - otlp/spanmetrics: - protocols: - grpc: - endpoint: "localhost:65535" - -exporters: - prometheus: - endpoint: "0.0.0.0:8889" - - otlp: - endpoint: jaeger:4317 - tls: - insecure: true - -processors: - batch: - spanmetrics: - metrics_exporter: prometheus - -service: - pipelines: - traces: - receivers: [otlp, jaeger] - processors: [spanmetrics, batch] - exporters: [otlp] - # The exporter name in this pipeline must match the spanmetrics.metrics_exporter name. - # The receiver is just a dummy and never used; added to pass validation requiring at least one receiver in a pipeline. - metrics/spanmetrics: - receivers: [otlp/spanmetrics] - exporters: [prometheus] diff --git a/pkg/prometheus/config/config.go b/pkg/prometheus/config/config.go index e316df30c90..122b831dea1 100644 --- a/pkg/prometheus/config/config.go +++ b/pkg/prometheus/config/config.go @@ -28,9 +28,8 @@ type Configuration struct { TokenFilePath string TokenOverrideFromContext bool - SupportSpanmetricsConnector bool - MetricNamespace string - LatencyUnit string - NormalizeCalls bool - NormalizeDuration bool + MetricNamespace string + LatencyUnit string + NormalizeCalls bool + NormalizeDuration bool } diff --git a/plugin/metrics/prometheus/factory_test.go b/plugin/metrics/prometheus/factory_test.go index db1374f280d..dc03edc2304 100644 --- a/plugin/metrics/prometheus/factory_test.go +++ b/plugin/metrics/prometheus/factory_test.go @@ -53,22 +53,11 @@ func TestWithDefaultConfiguration(t *testing.T) { assert.Equal(t, "http://localhost:9090", f.options.Primary.ServerURL) assert.Equal(t, 30*time.Second, f.options.Primary.ConnectTimeout) - assert.True(t, f.options.Primary.SupportSpanmetricsConnector) assert.Empty(t, f.options.Primary.MetricNamespace) assert.Equal(t, "ms", f.options.Primary.LatencyUnit) } func TestWithConfiguration(t *testing.T) { - t.Run("still supports the deprecated spanmetrics processor", func(t *testing.T) { - f := NewFactory() - v, command := config.Viperize(f.AddFlags) - err := command.ParseFlags([]string{ - "--prometheus.query.support-spanmetrics-connector=false", - }) - require.NoError(t, err) - f.InitFromViper(v, zap.NewNop()) - assert.False(t, f.options.Primary.SupportSpanmetricsConnector) - }) t.Run("with custom configuration and no space in token file path", func(t *testing.T) { f := NewFactory() v, command := config.Viperize(f.AddFlags) @@ -99,13 +88,11 @@ func TestWithConfiguration(t *testing.T) { f := NewFactory() v, command := config.Viperize(f.AddFlags) err := command.ParseFlags([]string{ - "--prometheus.query.support-spanmetrics-connector=true", "--prometheus.query.namespace=mynamespace", "--prometheus.query.duration-unit=ms", }) require.NoError(t, err) f.InitFromViper(v, zap.NewNop()) - assert.True(t, f.options.Primary.SupportSpanmetricsConnector) assert.Equal(t, "mynamespace", f.options.Primary.MetricNamespace) assert.Equal(t, "ms", f.options.Primary.LatencyUnit) }) diff --git a/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain.go b/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain.go index 4694a2ef2f7..65082c9d257 100644 --- a/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain.go +++ b/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain.go @@ -31,8 +31,8 @@ type Translator struct { // New returns a new Translator. func New(spanNameLabel string) Translator { return Translator{ - // "operator" is the label name that Jaeger UI expects. - labelMap: map[string]string{spanNameLabel: "operation"}, + // "span_name" is the label name that Jaeger UI expects. + labelMap: map[string]string{spanNameLabel: "span_name"}, } } diff --git a/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain_test.go b/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain_test.go index ad14ed3f4c2..25b8bf09c12 100644 --- a/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain_test.go +++ b/plugin/metrics/prometheus/metricsstore/dbmodel/to_domain_test.go @@ -48,7 +48,7 @@ func TestToDomainMetricsFamily(t *testing.T) { wantMetricLabels := map[string]string{ "label_key": "label_value", - "operation": "span_name_value", // assert the name is translated to a Jaeger-friendly label. + "span_name": "span_name_value", // assert the name is translated to a Jaeger-friendly label. } assert.Len(t, mf.Metrics, 1) for _, ml := range mf.Metrics[0].Labels { diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index fc86bd02019..253a09ef8c3 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -90,10 +90,7 @@ func NewMetricsReader(cfg config.Configuration, logger *zap.Logger, tracer trace return nil, fmt.Errorf("failed to initialize prometheus client: %w", err) } - operationLabel := "operation" - if cfg.SupportSpanmetricsConnector { - operationLabel = "span_name" - } + operationLabel := "span_name" mr := &MetricsReader{ client: promapi.NewAPI(client), @@ -134,11 +131,7 @@ func (m MetricsReader) GetLatencies(ctx context.Context, requestParams *metricss } func buildFullLatencyMetricName(cfg config.Configuration) string { - metricName := "latency" - if !cfg.SupportSpanmetricsConnector { - return metricName - } - metricName = "duration" + metricName := "duration" if cfg.MetricNamespace != "" { metricName = cfg.MetricNamespace + "_" + metricName @@ -181,10 +174,6 @@ func (m MetricsReader) GetCallRates(ctx context.Context, requestParams *metricss func buildFullCallsMetricName(cfg config.Configuration) string { metricName := "calls" - if !cfg.SupportSpanmetricsConnector { - return metricName - } - if cfg.MetricNamespace != "" { metricName = cfg.MetricNamespace + "_" + metricName } diff --git a/plugin/metrics/prometheus/metricsstore/reader_test.go b/plugin/metrics/prometheus/metricsstore/reader_test.go index c5199a684ac..ffa77c12078 100644 --- a/plugin/metrics/prometheus/metricsstore/reader_test.go +++ b/plugin/metrics/prometheus/metricsstore/reader_test.go @@ -16,6 +16,7 @@ package metricsstore import ( "context" + "fmt" "io" "net" "net/http" @@ -61,9 +62,8 @@ const defaultTimeout = 30 * time.Second // defaultConfig should consist of the default values for the prometheus.query.* command line options. var defaultConfig = config.Configuration{ - SupportSpanmetricsConnector: false, - MetricNamespace: "", - LatencyUnit: "ms", + MetricNamespace: "", + LatencyUnit: "ms", } func tracerProvider(t *testing.T) (trace.TracerProvider, *tracetest.InMemoryExporter, func()) { @@ -172,7 +172,7 @@ func TestGetLatencies(t *testing.T) { wantLabels: map[string]string{ "service_name": "emailservice", }, - wantPromQlQuery: `histogram_quantile(0.95, sum(rate(latency_bucket{service_name =~ "emailservice", ` + + wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_bucket{service_name =~ "emailservice", ` + `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,le))`, }, { @@ -183,11 +183,11 @@ func TestGetLatencies(t *testing.T) { wantName: "service_operation_latencies", wantDescription: "0.95th quantile latency, grouped by service & operation", wantLabels: map[string]string{ - "operation": "/OrderResult", + "span_name": "/OrderResult", "service_name": "emailservice", }, - wantPromQlQuery: `histogram_quantile(0.95, sum(rate(latency_bucket{service_name =~ "emailservice", ` + - `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,operation,le))`, + wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_bucket{service_name =~ "emailservice", ` + + `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,span_name,le))`, }, { name: "two services and span kinds result in regex 'or' symbol in query", @@ -199,16 +199,15 @@ func TestGetLatencies(t *testing.T) { wantLabels: map[string]string{ "service_name": "emailservice", }, - wantPromQlQuery: `histogram_quantile(0.95, sum(rate(latency_bucket{service_name =~ "frontend|emailservice", ` + + wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_bucket{service_name =~ "frontend|emailservice", ` + `span_kind =~ "SPAN_KIND_SERVER|SPAN_KIND_CLIENT"}[10m])) by (service_name,le))`, }, { - name: "enable support for spanmetrics connector with normalized metric name", + name: "enable support for spanmetrics connector with a namespace", serviceNames: []string{"emailservice"}, spanKinds: []string{"SPAN_KIND_SERVER"}, groupByOperation: true, updateConfig: func(cfg config.Configuration) config.Configuration { - cfg.SupportSpanmetricsConnector = true cfg.MetricNamespace = "span_metrics" cfg.LatencyUnit = "s" return cfg @@ -216,6 +215,7 @@ func TestGetLatencies(t *testing.T) { wantName: "service_operation_latencies", wantDescription: "0.95th quantile latency, grouped by service & operation", wantLabels: map[string]string{ + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `histogram_quantile(0.95, sum(rate(span_metrics_duration_bucket{service_name =~ "emailservice", ` + @@ -227,7 +227,6 @@ func TestGetLatencies(t *testing.T) { spanKinds: []string{"SPAN_KIND_SERVER"}, groupByOperation: true, updateConfig: func(cfg config.Configuration) config.Configuration { - cfg.SupportSpanmetricsConnector = true cfg.NormalizeDuration = true cfg.LatencyUnit = "s" return cfg @@ -235,6 +234,7 @@ func TestGetLatencies(t *testing.T) { wantName: "service_operation_latencies", wantDescription: "0.95th quantile latency, grouped by service & operation", wantLabels: map[string]string{ + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_seconds_bucket{service_name =~ "emailservice", ` + @@ -257,7 +257,7 @@ func TestGetLatencies(t *testing.T) { m, err := reader.GetLatencies(context.Background(), ¶ms) require.NoError(t, err) - assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription) + assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription) assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported") }) } @@ -286,11 +286,11 @@ func TestGetCallRates(t *testing.T) { wantName: "service_operation_call_rate", wantDescription: "calls/sec, grouped by service & operation", wantLabels: map[string]string{ - "operation": "/OrderResult", + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `sum(rate(calls{service_name =~ "emailservice", ` + - `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,operation)`, + `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,span_name)`, }, { name: "two services and span kinds result in regex 'or' symbol in query", @@ -311,13 +311,13 @@ func TestGetCallRates(t *testing.T) { spanKinds: []string{"SPAN_KIND_SERVER"}, groupByOperation: true, updateConfig: func(cfg config.Configuration) config.Configuration { - cfg.SupportSpanmetricsConnector = true cfg.MetricNamespace = "span_metrics" return cfg }, wantName: "service_operation_call_rate", wantDescription: "calls/sec, grouped by service & operation", wantLabels: map[string]string{ + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `sum(rate(span_metrics_calls{service_name =~ "emailservice", ` + @@ -329,13 +329,13 @@ func TestGetCallRates(t *testing.T) { spanKinds: []string{"SPAN_KIND_SERVER"}, groupByOperation: true, updateConfig: func(cfg config.Configuration) config.Configuration { - cfg.SupportSpanmetricsConnector = true cfg.NormalizeCalls = true return cfg }, wantName: "service_operation_call_rate", wantDescription: "calls/sec, grouped by service & operation", wantLabels: map[string]string{ + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `sum(rate(calls_total{service_name =~ "emailservice", ` + @@ -357,7 +357,7 @@ func TestGetCallRates(t *testing.T) { m, err := reader.GetCallRates(context.Background(), ¶ms) require.NoError(t, err) - assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription) + assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription) assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported") }) } @@ -387,12 +387,12 @@ func TestGetErrorRates(t *testing.T) { wantName: "service_operation_error_rate", wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation", wantLabels: map[string]string{ - "operation": "/OrderResult", + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `sum(rate(calls{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` + - `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,operation) / ` + - `sum(rate(calls{service_name =~ "emailservice", span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,operation)`, + `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,span_name) / ` + + `sum(rate(calls{service_name =~ "emailservice", span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name,span_name)`, }, { name: "two services and span kinds result in regex 'or' symbol in query", @@ -414,7 +414,6 @@ func TestGetErrorRates(t *testing.T) { spanKinds: []string{"SPAN_KIND_SERVER"}, groupByOperation: false, updateConfig: func(cfg config.Configuration) config.Configuration { - cfg.SupportSpanmetricsConnector = false cfg.MetricNamespace = "span_metrics" cfg.NormalizeCalls = true return cfg @@ -424,9 +423,9 @@ func TestGetErrorRates(t *testing.T) { wantLabels: map[string]string{ "service_name": "emailservice", }, - wantPromQlQuery: `sum(rate(calls{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` + + wantPromQlQuery: `sum(rate(span_metrics_calls_total{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` + `span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name) / ` + - `sum(rate(calls{service_name =~ "emailservice", span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name)`, + `sum(rate(span_metrics_calls_total{service_name =~ "emailservice", span_kind =~ "SPAN_KIND_SERVER"}[10m])) by (service_name)`, }, { name: "enable support for spanmetrics connector with a metric namespace", @@ -434,13 +433,13 @@ func TestGetErrorRates(t *testing.T) { spanKinds: []string{"SPAN_KIND_SERVER"}, groupByOperation: true, updateConfig: func(cfg config.Configuration) config.Configuration { - cfg.SupportSpanmetricsConnector = true cfg.MetricNamespace = "span_metrics" return cfg }, wantName: "service_operation_error_rate", wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation", wantLabels: map[string]string{ + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `sum(rate(span_metrics_calls{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` + @@ -453,13 +452,13 @@ func TestGetErrorRates(t *testing.T) { spanKinds: []string{"SPAN_KIND_SERVER"}, groupByOperation: true, updateConfig: func(cfg config.Configuration) config.Configuration { - cfg.SupportSpanmetricsConnector = true cfg.NormalizeCalls = true return cfg }, wantName: "service_operation_error_rate", wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation", wantLabels: map[string]string{ + "span_name": "/OrderResult", "service_name": "emailservice", }, wantPromQlQuery: `sum(rate(calls_total{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` + @@ -482,7 +481,7 @@ func TestGetErrorRates(t *testing.T) { m, err := reader.GetErrorRates(context.Background(), ¶ms) require.NoError(t, err) - assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription) + assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription) assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported") }) } @@ -699,9 +698,8 @@ func TestInvalidLatencyUnit(t *testing.T) { tracer, _, closer := tracerProvider(t) defer closer() cfg := config.Configuration{ - SupportSpanmetricsConnector: true, - NormalizeDuration: true, - LatencyUnit: "something invalid", + NormalizeDuration: true, + LatencyUnit: "something invalid", } _, _ = NewMetricsReader(cfg, zap.NewNop(), tracer) } @@ -900,8 +898,8 @@ func startMockPrometheusServer(t *testing.T, wantPromQlQuery string, wantWarning assert.Equal(t, wantPromQlQuery, promQuery) mockResponsePayloadFile := "testdata/service_datapoint_response.json" - if strings.Contains(promQuery, "by (service_name,operation") { - mockResponsePayloadFile = "testdata/service_operation_datapoint_response.json" + if strings.Contains(promQuery, "by (service_name,span_name") { + mockResponsePayloadFile = "testdata/service_span_name_datapoint_response.json" } sendResponse(t, w, mockResponsePayloadFile) })) @@ -947,13 +945,18 @@ func prepareMetricsReaderAndServer(t *testing.T, config config.Configuration, wa return reader, mockPrometheus } -func assertMetrics(t *testing.T, gotMetrics *metrics.MetricFamily, wantLabels map[string]string, wantName, wantDescription string) { +func assertMetrics(t *testing.T, testName string, gotMetrics *metrics.MetricFamily, wantLabels map[string]string, wantName, wantDescription string) { assert.Len(t, gotMetrics.Metrics, 1) assert.Equal(t, wantName, gotMetrics.Name) assert.Equal(t, wantDescription, gotMetrics.Help) mps := gotMetrics.Metrics[0].MetricPoints assert.Len(t, mps, 1) + // logging for expected and actual labels + fmt.Printf("Test Name: %s\n", testName) + fmt.Printf("Expected labels: %v\n", wantLabels) + fmt.Printf("Actual labels: %v\n", gotMetrics.Metrics[0].Labels) + // There is no guaranteed order of labels, so we need to take the approach of using a map of expected values. labels := gotMetrics.Metrics[0].Labels assert.Equal(t, len(wantLabels), len(labels)) @@ -963,6 +966,11 @@ func assertMetrics(t *testing.T, gotMetrics *metrics.MetricFamily, wantLabels ma delete(wantLabels, l.Name) } assert.Empty(t, wantLabels) + + // Additional logging to show that all expected labels were found and matched + fmt.Printf("Remaining expected labels after matching: %v\n", wantLabels) + fmt.Printf("\n") + assert.Equal(t, int64(1620351786), mps[0].Timestamp.GetSeconds()) actualVal := mps[0].Value.(*metrics.MetricPoint_GaugeValue).GaugeValue.Value.(*metrics.GaugeValue_DoubleValue).DoubleValue diff --git a/plugin/metrics/prometheus/metricsstore/testdata/service_operation_datapoint_response.json b/plugin/metrics/prometheus/metricsstore/testdata/service_span_name_datapoint_response.json similarity index 88% rename from plugin/metrics/prometheus/metricsstore/testdata/service_operation_datapoint_response.json rename to plugin/metrics/prometheus/metricsstore/testdata/service_span_name_datapoint_response.json index bfe13afc8fd..8fb412b9d1d 100644 --- a/plugin/metrics/prometheus/metricsstore/testdata/service_operation_datapoint_response.json +++ b/plugin/metrics/prometheus/metricsstore/testdata/service_span_name_datapoint_response.json @@ -5,7 +5,7 @@ "result": [ { "metric": { - "operation": "/OrderResult", + "span_name": "/OrderResult", "service_name": "emailservice" }, "values": [ diff --git a/plugin/metrics/prometheus/options.go b/plugin/metrics/prometheus/options.go index db516bc24f3..a9f8f98956b 100644 --- a/plugin/metrics/prometheus/options.go +++ b/plugin/metrics/prometheus/options.go @@ -17,7 +17,6 @@ package prometheus import ( "flag" "fmt" - "log" "strings" "time" @@ -33,11 +32,10 @@ const ( suffixTokenFilePath = ".token-file" suffixOverrideFromContext = ".token-override-from-context" - suffixSupportSpanmetricsConnector = ".query.support-spanmetrics-connector" - suffixMetricNamespace = ".query.namespace" - suffixLatencyUnit = ".query.duration-unit" - suffixNormalizeCalls = ".query.normalize-calls" - suffixNormalizeDuration = ".query.normalize-duration" + suffixMetricNamespace = ".query.namespace" + suffixLatencyUnit = ".query.duration-unit" + suffixNormalizeCalls = ".query.normalize-calls" + suffixNormalizeDuration = ".query.normalize-duration" defaultServerURL = "http://localhost:9090" defaultConnectTimeout = 30 * time.Second @@ -48,8 +46,6 @@ const ( defaultLatencyUnit = "ms" defaultNormalizeCalls = false defaultNormalizeDuration = false - - deprecatedSpanMetricsProcessor = "(deprecated, will be removed after 2024-01-01 or in release v1.53.0, whichever is later) " ) type namespaceConfig struct { @@ -68,11 +64,10 @@ func NewOptions(primaryNamespace string) *Options { ServerURL: defaultServerURL, ConnectTimeout: defaultConnectTimeout, - SupportSpanmetricsConnector: defaultSupportSpanmetricsConnector, - MetricNamespace: defaultMetricNamespace, - LatencyUnit: defaultLatencyUnit, - NormalizeCalls: defaultNormalizeCalls, - NormalizeDuration: defaultNormalizeCalls, + MetricNamespace: defaultMetricNamespace, + LatencyUnit: defaultLatencyUnit, + NormalizeCalls: defaultNormalizeCalls, + NormalizeDuration: defaultNormalizeCalls, } return &Options{ @@ -94,10 +89,6 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) { "The path to a file containing the bearer token which will be included when executing queries against the Prometheus API.") flagSet.Bool(nsConfig.namespace+suffixOverrideFromContext, true, "Whether the bearer token should be overridden from context (incoming request)") - flagSet.Bool( - nsConfig.namespace+suffixSupportSpanmetricsConnector, - defaultSupportSpanmetricsConnector, - deprecatedSpanMetricsProcessor+" Controls whether the metrics queries should match the OpenTelemetry Collector's spanmetrics connector naming (when true) or spanmetrics processor naming (when false).") flagSet.String(nsConfig.namespace+suffixMetricNamespace, defaultMetricNamespace, `The metric namespace that is prefixed to the metric name. A '.' separator will be added between `+ `the namespace and the metric name.`) @@ -127,10 +118,6 @@ func (opt *Options) InitFromViper(v *viper.Viper) error { cfg.ConnectTimeout = v.GetDuration(cfg.namespace + suffixConnectTimeout) cfg.TokenFilePath = v.GetString(cfg.namespace + suffixTokenFilePath) - cfg.SupportSpanmetricsConnector = v.GetBool(cfg.namespace + suffixSupportSpanmetricsConnector) - if !cfg.SupportSpanmetricsConnector { - log.Printf("using Spanmetrics Processor's metrics naming conventions " + deprecatedSpanMetricsProcessor) - } cfg.MetricNamespace = v.GetString(cfg.namespace + suffixMetricNamespace) cfg.LatencyUnit = v.GetString(cfg.namespace + suffixLatencyUnit) cfg.NormalizeCalls = v.GetBool(cfg.namespace + suffixNormalizeCalls)