Merge remote-tracking branch 'origin/main' into upgrade-hotrod-to-otel

Signed-off-by: Afzal Ansari <[email protected]>

.github/workflows/ci-release.yml

@@ -89,6 +89,6 @@ jobs:
         QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
 
     - name: SBOM Generation
-      uses: anchore/sbom-action@422cb34a0f8b599678c41b21163ea6088edb2624
+      uses: anchore/sbom-action@78fc58e266e87a38d4194b2137a3d4e9bcaf7ca1
       with:
         artifact-name: jaeger-SBOM.spdx.json

CHANGELOG.md

@@ -7,6 +7,13 @@ next release (yyyy-mm-dd)
 
 #### ⛔ Breaking Changes
 
+* [SPM] Due to a breaking change in OpenTelemetry's prometheus exporter ([details](https://github.com/open-telemetry/opentelemetry-collector-contrib/releases/tag/v0.80.0))
+  metric names will no longer be normalized by default, meaning that the expected metric names would be `calls` and
+  `duration_*`. Backwards compatibility with older OpenTelemetry Collector versions can be achieved through the following flags:
+  * `prometheus.query.normalize-calls`: If true, normalizes the "calls" metric name. e.g. "calls_total".
+  * `prometheus.query.normalize-duration`: If true, normalizes the "duration" metric name to include the duration units. e.g. "duration_milliseconds_bucket".
+
+
 #### New Features
 
 #### Bug fixes, Minor Improvements

cmd/collector/app/server/http_test.go

@@ -26,7 +26,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
-	"go.uber.org/zap/zaptest"
 
 	"github.com/jaegertracing/jaeger/cmd/collector/app/handler"
 	"github.com/jaegertracing/jaeger/internal/metricstest"
@@ -94,7 +93,6 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 		clientTLS         tlscfg.Options
 		expectError       bool
 		expectClientError bool
-		expectServerFail  bool
 	}{
 		{
 			name: "should fail with TLS client to untrusted TLS server",
@@ -109,7 +107,6 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 			},
 			expectError:       true,
 			expectClientError: true,
-			expectServerFail:  false,
 		},
 		{
 			name: "should fail with TLS client to trusted TLS server with incorrect hostname",
@@ -125,7 +122,6 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 			},
 			expectError:       true,
 			expectClientError: true,
-			expectServerFail:  false,
 		},
 		{
 			name: "should pass with TLS client to trusted TLS server with correct hostname",
@@ -139,9 +135,6 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
 				ServerName: "example.com",
 			},
-			expectError:       false,
-			expectClientError: false,
-			expectServerFail:  false,
 		},
 		{
 			name: "should fail with TLS client without cert to trusted TLS server requiring cert",
@@ -156,8 +149,6 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
 				ServerName: "example.com",
 			},
-			expectError:       false,
-			expectServerFail:  false,
 			expectClientError: true,
 		},
 		{
@@ -175,9 +166,6 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 				CertPath:   testCertKeyLocation + "/example-client-cert.pem",
 				KeyPath:    testCertKeyLocation + "/example-client-key.pem",
 			},
-			expectError:       false,
-			expectServerFail:  false,
-			expectClientError: false,
 		},
 		{
 			name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA",
@@ -194,15 +182,15 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 				CertPath:   testCertKeyLocation + "/example-client-cert.pem",
 				KeyPath:    testCertKeyLocation + "/example-client-key.pem",
 			},
-			expectError:       false,
-			expectServerFail:  false,
 			expectClientError: true,
 		},
 	}
 
 	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
-			logger := zaptest.NewLogger(t)
+			// Cannot reliably use zaptest.NewLogger(t) because it causes race condition
+			// See https://github.com/jaegertracing/jaeger/issues/4497.
+			logger := zap.NewNop()
 			params := &HTTPServerParams{
 				HostPort:       fmt.Sprintf(":%d", ports.CollectorHTTP),
 				Handler:        handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}),
@@ -214,14 +202,12 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 			}
 
 			server, err := StartHTTPServer(params)
-
-			if test.expectServerFail {
-				require.Error(t, err)
-			}
-			defer server.Close()
-
 			require.NoError(t, err)
-			clientTLSCfg, err0 := test.clientTLS.Config(zap.NewNop())
+			defer func() {
+				assert.NoError(t, server.Close())
+			}()
+
+			clientTLSCfg, err0 := test.clientTLS.Config(logger)
 			require.NoError(t, err0)
 			dialer := &net.Dialer{Timeout: 2 * time.Second}
 			conn, clientError := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.CollectorHTTP), clientTLSCfg)
@@ -260,7 +246,7 @@ func TestSpanCollectorHTTPS(t *testing.T) {
 }
 
 func TestStartHTTPServerParams(t *testing.T) {
-	logger := zaptest.NewLogger(t)
+	logger := zap.NewNop()
 	params := &HTTPServerParams{
 		HostPort:          fmt.Sprintf(":%d", ports.CollectorHTTP),
 		Handler:           handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}),

docker-compose/monitor/Makefile

@@ -18,7 +18,7 @@ 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 = latest
+_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:

examples/hotrod/pkg/log/factory.go

@@ -18,7 +18,6 @@ package log
 import (
 	"context"
 
-	ot "github.com/opentracing/opentracing-go"
 	"go.opentelemetry.io/otel/trace"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
@@ -44,16 +43,12 @@ func (b Factory) Bg() Logger {
 // contains an OpenTracing span, all logging calls are also
 // echo-ed into the span.
 func (b Factory) For(ctx context.Context) Logger {
-	if otSpan := ot.SpanFromContext(ctx); otSpan != nil {
-		logger := spanLogger{span: otSpan, logger: b.logger}
-
-		if otelSpan := trace.SpanFromContext(ctx); otelSpan != nil {
-			logger.spanFields = []zapcore.Field{
-				zap.String("trace_id", otelSpan.SpanContext().TraceID().String()),
-				zap.String("span_id", otelSpan.SpanContext().SpanID().String()),
-			}
+	if otelSpan := trace.SpanFromContext(ctx); otelSpan != nil {
+		logger := spanLogger{span: otelSpan, logger: b.logger}
+		logger.spanFields = []zapcore.Field{
+			zap.String("trace_id", otelSpan.SpanContext().TraceID().String()),
+			zap.String("span_id", otelSpan.SpanContext().SpanID().String()),
 		}
-
 		return logger
 	}
 	return b.Bg()

examples/hotrod/pkg/log/spanlogger.go

@@ -16,23 +16,24 @@
 package log
 
 import (
+	"fmt"
 	"time"
 
-	"github.com/opentracing/opentracing-go"
-	tag "github.com/opentracing/opentracing-go/ext"
-	"github.com/opentracing/opentracing-go/log"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/trace"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 )
 
 type spanLogger struct {
 	logger     *zap.Logger
-	span       opentracing.Span
+	span       trace.Span
 	spanFields []zapcore.Field
 }
 
 func (sl spanLogger) Debug(msg string, fields ...zapcore.Field) {
-	sl.logToSpan("Debug", msg, fields...)
+	sl.logToSpan("debug", msg, fields...)
 	sl.logger.Debug(msg, append(sl.spanFields, fields...)...)
 }
 
@@ -48,7 +49,7 @@ func (sl spanLogger) Error(msg string, fields ...zapcore.Field) {
 
 func (sl spanLogger) Fatal(msg string, fields ...zapcore.Field) {
 	sl.logToSpan("fatal", msg, fields...)
-	tag.Error.Set(sl.span, true)
+	sl.span.SetStatus(codes.Error, msg)
 	sl.logger.Fatal(msg, append(sl.spanFields, fields...)...)
 }
 
@@ -57,99 +58,119 @@ func (sl spanLogger) With(fields ...zapcore.Field) Logger {
 	return spanLogger{logger: sl.logger.With(fields...), span: sl.span, spanFields: sl.spanFields}
 }
 
-func (sl spanLogger) logToSpan(level string, msg string, fields ...zapcore.Field) {
-	// TODO rather than always converting the fields, we could wrap them into a lazy logger
-	fa := fieldAdapter(make([]log.Field, 0, 2+len(fields)))
-	fa = append(fa, log.String("event", msg))
-	fa = append(fa, log.String("level", level))
+func (sl spanLogger) logToSpan(level, msg string, fields ...zapcore.Field) {
+	fields = append(fields, zap.String("level", level))
+	sl.span.AddEvent(
+		msg,
+		trace.WithAttributes(logFieldsToOTelAttrs(fields)...),
+	)
+}
+
+func logFieldsToOTelAttrs(fields []zapcore.Field) []attribute.KeyValue {
+	encoder := &bridgeFieldEncoder{}
 	for _, field := range fields {
-		field.AddTo(&fa)
+		field.AddTo(encoder)
 	}
-	sl.span.LogFields(fa...)
+	return encoder.pairs
 }
 
-type fieldAdapter []log.Field
+type bridgeFieldEncoder struct {
+	pairs []attribute.KeyValue
+}
 
-func (fa *fieldAdapter) AddBool(key string, value bool) {
-	*fa = append(*fa, log.Bool(key, value))
+func (e *bridgeFieldEncoder) AddArray(key string, marshaler zapcore.ArrayMarshaler) error {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(marshaler)))
+	return nil
 }
 
-func (fa *fieldAdapter) AddFloat64(key string, value float64) {
-	*fa = append(*fa, log.Float64(key, value))
+func (e *bridgeFieldEncoder) AddObject(key string, marshaler zapcore.ObjectMarshaler) error {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(marshaler)))
+	return nil
 }
 
-func (fa *fieldAdapter) AddFloat32(key string, value float32) {
-	*fa = append(*fa, log.Float64(key, float64(value)))
+func (e *bridgeFieldEncoder) AddBinary(key string, value []byte) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(value)))
 }
 
-func (fa *fieldAdapter) AddInt(key string, value int) {
-	*fa = append(*fa, log.Int(key, value))
+func (e *bridgeFieldEncoder) AddByteString(key string, value []byte) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(value)))
 }
 
-func (fa *fieldAdapter) AddInt64(key string, value int64) {
-	*fa = append(*fa, log.Int64(key, value))
+func (e *bridgeFieldEncoder) AddBool(key string, value bool) {
+	e.pairs = append(e.pairs, attribute.Bool(key, value))
 }
 
-func (fa *fieldAdapter) AddInt32(key string, value int32) {
-	*fa = append(*fa, log.Int64(key, int64(value)))
+func (e *bridgeFieldEncoder) AddComplex128(key string, value complex128) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(value)))
 }
 
-func (fa *fieldAdapter) AddInt16(key string, value int16) {
-	*fa = append(*fa, log.Int64(key, int64(value)))
+func (e *bridgeFieldEncoder) AddComplex64(key string, value complex64) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(value)))
 }
 
-func (fa *fieldAdapter) AddInt8(key string, value int8) {
-	*fa = append(*fa, log.Int64(key, int64(value)))
+func (e *bridgeFieldEncoder) AddDuration(key string, value time.Duration) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(value)))
 }
 
-func (fa *fieldAdapter) AddUint(key string, value uint) {
-	*fa = append(*fa, log.Uint64(key, uint64(value)))
+func (e *bridgeFieldEncoder) AddFloat64(key string, value float64) {
+	e.pairs = append(e.pairs, attribute.Float64(key, value))
 }
 
-func (fa *fieldAdapter) AddUint64(key string, value uint64) {
-	*fa = append(*fa, log.Uint64(key, value))
+func (e *bridgeFieldEncoder) AddFloat32(key string, value float32) {
+	e.pairs = append(e.pairs, attribute.Float64(key, float64(value)))
 }
 
-func (fa *fieldAdapter) AddUint32(key string, value uint32) {
-	*fa = append(*fa, log.Uint64(key, uint64(value)))
+func (e *bridgeFieldEncoder) AddInt(key string, value int) {
+	e.pairs = append(e.pairs, attribute.Int(key, value))
 }
 
-func (fa *fieldAdapter) AddUint16(key string, value uint16) {
-	*fa = append(*fa, log.Uint64(key, uint64(value)))
+func (e *bridgeFieldEncoder) AddInt64(key string, value int64) {
+	e.pairs = append(e.pairs, attribute.Int64(key, value))
 }
 
-func (fa *fieldAdapter) AddUint8(key string, value uint8) {
-	*fa = append(*fa, log.Uint64(key, uint64(value)))
+func (e *bridgeFieldEncoder) AddInt32(key string, value int32) {
+	e.pairs = append(e.pairs, attribute.Int64(key, int64(value)))
 }
 
-func (fa *fieldAdapter) AddUintptr(key string, value uintptr)                        {}
-func (fa *fieldAdapter) AddArray(key string, marshaler zapcore.ArrayMarshaler) error { return nil }
-func (fa *fieldAdapter) AddComplex128(key string, value complex128)                  {}
-func (fa *fieldAdapter) AddComplex64(key string, value complex64)                    {}
-func (fa *fieldAdapter) AddObject(key string, value zapcore.ObjectMarshaler) error   { return nil }
-func (fa *fieldAdapter) AddReflected(key string, value interface{}) error            { return nil }
-func (fa *fieldAdapter) OpenNamespace(key string)                                    {}
+func (e *bridgeFieldEncoder) AddInt16(key string, value int16) {
+	e.pairs = append(e.pairs, attribute.Int64(key, int64(value)))
+}
 
-func (fa *fieldAdapter) AddDuration(key string, value time.Duration) {
-	// TODO inefficient
-	*fa = append(*fa, log.String(key, value.String()))
+func (e *bridgeFieldEncoder) AddInt8(key string, value int8) {
+	e.pairs = append(e.pairs, attribute.Int64(key, int64(value)))
 }
 
-func (fa *fieldAdapter) AddTime(key string, value time.Time) {
-	// TODO inefficient
-	*fa = append(*fa, log.String(key, value.String()))
+func (e *bridgeFieldEncoder) AddString(key, value string) {
+	e.pairs = append(e.pairs, attribute.String(key, value))
 }
 
-func (fa *fieldAdapter) AddBinary(key string, value []byte) {
-	*fa = append(*fa, log.Object(key, value))
+func (e *bridgeFieldEncoder) AddTime(key string, value time.Time) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(value)))
 }
 
-func (fa *fieldAdapter) AddByteString(key string, value []byte) {
-	*fa = append(*fa, log.Object(key, value))
+func (e *bridgeFieldEncoder) AddUint(key string, value uint) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprintf("%d", value)))
 }
 
-func (fa *fieldAdapter) AddString(key, value string) {
-	if key != "" && value != "" {
-		*fa = append(*fa, log.String(key, value))
-	}
+func (e *bridgeFieldEncoder) AddUint64(key string, value uint64) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprintf("%d", value)))
+}
+
+func (e *bridgeFieldEncoder) AddUint32(key string, value uint32) {
+	e.pairs = append(e.pairs, attribute.Int64(key, int64(value)))
 }
+
+func (e *bridgeFieldEncoder) AddUint16(key string, value uint16) {
+	e.pairs = append(e.pairs, attribute.Int64(key, int64(value)))
+}
+
+func (e *bridgeFieldEncoder) AddUint8(key string, value uint8) {
+	e.pairs = append(e.pairs, attribute.Int64(key, int64(value)))
+}
+
+func (e *bridgeFieldEncoder) AddUintptr(key string, value uintptr) {
+	e.pairs = append(e.pairs, attribute.String(key, fmt.Sprint(value)))
+}
+
+func (e *bridgeFieldEncoder) AddReflected(key string, value interface{}) error { return nil }
+func (e *bridgeFieldEncoder) OpenNamespace(key string)                         {}