Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support spanmetrics connector by default #4704

Merged
merged 7 commits into from
Aug 29, 2023
Merged

Support spanmetrics connector by default #4704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2448662
Default to spanmetrics connector
albertteoh Aug 25, 2023
e7ac86f
Add connector config as defaults
albertteoh Aug 28, 2023
98ddd13
Merge branch 'default-connector' into improve-local-dev
albertteoh Aug 28, 2023
ec4de57
Default to spanmetrics connector
albertteoh Aug 28, 2023
a8ec39c
Add breaking change in changelog
albertteoh Aug 28, 2023
e2dbe63
Update unit tests
albertteoh Aug 28, 2023
e926a47
Update CHANGELOG.md
albertteoh Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
33 changes: 11 additions & 22 deletions docker-compose/monitor/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docker-compose/monitor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:**
Expand Down
13 changes: 6 additions & 7 deletions docker-compose/monitor/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions docker-compose/monitor/otel-collector-config-connector.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ exporters:
prometheus:
endpoint: "0.0.0.0:8889"

jaeger:
endpoint: "jaeger:14250"
otlp:
endpoint: jaeger:4317
tls:
insecure: true

Expand All @@ -29,7 +29,7 @@ service:
traces:
receivers: [otlp, jaeger]
processors: [batch]
exporters: [spanmetrics, jaeger]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jaeger exporter is deprecated, hence the switch to otlp exporter.

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:
Expand Down
6 changes: 3 additions & 3 deletions docker-compose/monitor/otel-collector-config-processor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ exporters:
prometheus:
endpoint: "0.0.0.0:8889"

jaeger:
endpoint: "jaeger:14250"
otlp:
endpoint: jaeger:4317
tls:
insecure: true

Expand All @@ -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:
Expand Down
13 changes: 11 additions & 2 deletions plugin/metrics/prometheus/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions plugin/metrics/prometheus/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const (
defaultConnectTimeout = 30 * time.Second
defaultTokenFilePath = ""

defaultSupportSpanmetricsConnector = false
defaultSupportSpanmetricsConnector = true
defaultMetricNamespace = ""
defaultLatencyUnit = "ms"
defaultNormalizeCalls = false
Expand All @@ -64,7 +64,7 @@ func NewOptions(primaryNamespace string) *Options {
ServerURL: defaultServerURL,
ConnectTimeout: defaultConnectTimeout,

SupportSpanmetricsConnector: false,
SupportSpanmetricsConnector: defaultSupportSpanmetricsConnector,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main (breaking) change.

The other changes relate to local developer improvements with a similar theme of defaulting to the spanmetrics connector as well.

If we feel those changes should be decoupled into separate PRs, I'm happy to break them up.

MetricNamespace: defaultMetricNamespace,
LatencyUnit: defaultLatencyUnit,
NormalizeCalls: defaultNormalizeCalls,
Expand Down