Support spanmetrics connector by default (#4704)

## Which problem is this PR solving? - Supports the new spanmetrics connector by default. - Some local developer improvements. ## Description of the changes - Defaults the `PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR` parameter to `true`. - The next release will remove this configuration and not support the spanmetrics processor. - Removes the use of the deprecated jaeger exporter in OTEL config. For more details, please refer to: open-telemetry/opentelemetry-specification#2858. - Simplifies the Makefile by: - Providing defaults in the docker-compose.yml file to prefer the spanmetrics connector. - Removing the `run` target in favour of `docker compose up`. - Removing the `run-` prefix of the remaining targets. ## How was this change tested? - Tested the following commands to confirm metrics are visible in the Monitor tab: ``` $ make build $ make dev $ make dev-processor $ docker compose up # No longer works $ make run ``` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits ~- [] 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: albertteoh <[email protected]> Signed-off-by: Albert <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
jaegertracing · Aug 29, 2023 · 3a6d057 · 3a6d057
1 parent 7a6bae2
commit 3a6d057
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,7 @@ next release (yyyy-mm-dd)
 #### ⛔ Breaking Changes
 
 * Make OTLP the default exporter in HotROD ([@yurishkuro](https://github.com/yurishkuro) in [#4698](https://github.com/jaegertracing/jaeger/pull/4698))
+* [SPM] Support spanmetrics connector by default ([@albertteoh](https://github.com/albertteoh) in [#4704](https://github.com/jaegertracing/jaeger/pull/4704))
 
 #### New Features
 

diff --git a/docker-compose/monitor/Makefile b/docker-compose/monitor/Makefile
@@ -5,33 +5,22 @@ build: clean-jaeger
 	make build-all-in-one && \
 	make docker-images-jaeger-backend
 
-# run starts up the system required for SPM using the latest jaeger and otel images.
-.PHONY: run
-run: export JAEGER_IMAGE_TAG = latest
-run: _run-connector
-
-# run starts up the system required for SPM using the latest otel image and a development jaeger image.
+# starts up the system required for SPM using the latest otel image and a development jaeger image.
 # Note: the jaeger "dev" image can be built with "make build".
-.PHONY: run-dev
-run-dev: export JAEGER_IMAGE_TAG = dev
-run-dev: _run-connector
-
-# _run-connector is the base target to bring up the system required for SPM using the new OTEL spanmetrics connector.
-.PHONY: _run-connector
-_run-connector: export OTEL_IMAGE_TAG = 0.80.0
-_run-connector: export OTEL_CONFIG_SRC = ./otel-collector-config-connector.yml
-_run-connector: export PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR = true
-_run-connector:
+.PHONY: dev
+dev: export JAEGER_IMAGE_TAG = dev
+dev:
 	docker compose -f docker-compose.yml up
 
-# run the older spanmetrics processor setup, for example,
+# starts older spanmetrics processor setup, for example,
 # to test backwards compatibility of Jaeger with spanmetrics processor.
-.PHONY: run-dev-processor
-run-dev-processor: export JAEGER_IMAGE_TAG = dev
+.PHONY: dev-processor
+dev-processor: export JAEGER_IMAGE_TAG = dev
 # Fix to a version before the breaking changes were introduced.
-run-dev-processor: export OTEL_IMAGE_TAG = 0.70.0
-run-dev-processor: export OTEL_CONFIG_SRC = ./otel-collector-config-processor.yml
-run-dev-processor:
+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

diff --git a/docker-compose/monitor/README.md b/docker-compose/monitor/README.md
@@ -32,7 +32,7 @@ This brings up the system necessary to use the SPM feature locally.
 It uses the latest image tags from both Jaeger and OpenTelemetry.
 
 ```shell
-make run
+docker compose up
 ```
 
 **Tips:**

diff --git a/docker-compose/monitor/docker-compose.yml b/docker-compose/monitor/docker-compose.yml
@@ -3,25 +3,24 @@ services:
   jaeger:
     networks:
       - backend
-    image: jaegertracing/all-in-one:${JAEGER_IMAGE_TAG}
+    image: jaegertracing/all-in-one:${JAEGER_IMAGE_TAG:-latest}
     volumes:
       - "./jaeger-ui.json:/etc/jaeger/jaeger-ui.json"
     command: --query.ui-config /etc/jaeger/jaeger-ui.json
     environment:
       - METRICS_STORAGE_TYPE=prometheus
       - PROMETHEUS_SERVER_URL=http://prometheus:9090
-      - LOG_LEVEL=debug
-      - PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR=${PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR}
-      - PROMETHEUS_QUERY_NAMESPACE=${PROMETHEUS_QUERY_NAMESPACE}
-      - PROMETHEUS_QUERY_DURATION_UNIT=${PROMETHEUS_QUERY_DURATION_UNIT}
+      - PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR=${PROMETHEUS_QUERY_SUPPORT_SPANMETRICS_CONNECTOR:-true}
+      - PROMETHEUS_QUERY_NAMESPACE=${PROMETHEUS_QUERY_NAMESPACE:-}
+      - PROMETHEUS_QUERY_DURATION_UNIT=${PROMETHEUS_QUERY_DURATION_UNIT:-}
     ports:
       - "16686:16686"
   otel_collector:
     networks:
       - backend
-    image: otel/opentelemetry-collector-contrib:${OTEL_IMAGE_TAG}
+    image: otel/opentelemetry-collector-contrib:${OTEL_IMAGE_TAG:-0.80.0}
     volumes:
-      - ${OTEL_CONFIG_SRC}:/etc/otelcol/otel-collector-config.yml
+      - ${OTEL_CONFIG_SRC:-./otel-collector-config-connector.yml}:/etc/otelcol/otel-collector-config.yml
     command: --config /etc/otelcol/otel-collector-config.yml
     ports:
       - "4317:4317"

diff --git a/docker-compose/monitor/otel-collector-config-connector.yml b/docker-compose/monitor/otel-collector-config-connector.yml
@@ -13,8 +13,8 @@ exporters:
   prometheus:
     endpoint: "0.0.0.0:8889"
 
-  jaeger:
-    endpoint: "jaeger:14250"
+  otlp:
+    endpoint: jaeger:4317
     tls:
       insecure: true
 
@@ -29,7 +29,7 @@ service:
     traces:
       receivers: [otlp, jaeger]
       processors: [batch]
-      exporters: [spanmetrics, jaeger]
+      exporters: [spanmetrics, 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:

diff --git a/docker-compose/monitor/otel-collector-config-processor.yml b/docker-compose/monitor/otel-collector-config-processor.yml
@@ -19,8 +19,8 @@ exporters:
   prometheus:
     endpoint: "0.0.0.0:8889"
 
-  jaeger:
-    endpoint: "jaeger:14250"
+  otlp:
+    endpoint: jaeger:4317
     tls:
       insecure: true
 
@@ -34,7 +34,7 @@ service:
     traces:
       receivers: [otlp, jaeger]
       processors: [spanmetrics, batch]
-      exporters: [jaeger]
+      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:

diff --git a/plugin/metrics/prometheus/factory_test.go b/plugin/metrics/prometheus/factory_test.go
@@ -53,13 +53,22 @@ 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)
 
-	// Ensure backwards compatibility with OTEL's spanmetricsprocessor.
-	assert.False(t, f.options.Primary.SupportSpanmetricsConnector)
+	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)

diff --git a/plugin/metrics/prometheus/options.go b/plugin/metrics/prometheus/options.go
@@ -41,7 +41,7 @@ const (
 	defaultConnectTimeout = 30 * time.Second
 	defaultTokenFilePath  = ""
 
-	defaultSupportSpanmetricsConnector = false
+	defaultSupportSpanmetricsConnector = true
 	defaultMetricNamespace             = ""
 	defaultLatencyUnit                 = "ms"
 	defaultNormalizeCalls              = false
@@ -64,7 +64,7 @@ func NewOptions(primaryNamespace string) *Options {
 		ServerURL:      defaultServerURL,
 		ConnectTimeout: defaultConnectTimeout,
 
-		SupportSpanmetricsConnector: false,
+		SupportSpanmetricsConnector: defaultSupportSpanmetricsConnector,
 		MetricNamespace:             defaultMetricNamespace,
 		LatencyUnit:                 defaultLatencyUnit,
 		NormalizeCalls:              defaultNormalizeCalls,