feat(exporter): Jaeger and Zipkin exporter use logr as the logging …

…interface (#3500)

* feat(exporter): Jaeger and Zipkin exporter use `github.com/go-logr/logr` as the logging interface, and add the WithLogr option

* fix(exporter): reuse code

* fix(exporter): lint

* fix(exporter): add comment

* fix(changelog): update

CHANGELOG.md

@@ -46,6 +46,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The `Temporality(view.InstrumentKind) metricdata.Temporality` and `Aggregation(view.InstrumentKind) aggregation.Aggregation` methods are added to the `"go.opentelemetry.io/otel/exporters/otlp/otlpmetric".Client` interface. (#3260)
 - The `WithTemporalitySelector` and `WithAggregationSelector` `ReaderOption`s have been changed to `ManualReaderOption`s in the `go.opentelemetry.io/otel/sdk/metric` package. (#3260)
 - The periodic reader in the `go.opentelemetry.io/otel/sdk/metric` package now uses the temporality and aggregation selectors from its configured exporter instead of accepting them as options. (#3260)
+- Jaeger and Zipkin exporter use `github.com/go-logr/logr` as the logging interface, and add the `WithLogr` option. (#3497, #3500)
 
 ### Fixed
 

exporters/jaeger/agent.go

@@ -18,11 +18,12 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"log"
 	"net"
 	"strings"
 	"time"
 
+	"github.com/go-logr/logr"
+
 	genAgent "go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/agent"
 	gen "go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger"
 	"go.opentelemetry.io/otel/exporters/jaeger/internal/third_party/thrift/lib/go/thrift"
@@ -58,7 +59,7 @@ type agentClientUDPParams struct {
 	Host                     string
 	Port                     string
 	MaxPacketSize            int
-	Logger                   *log.Logger
+	Logger                   logr.Logger
 	AttemptReconnecting      bool
 	AttemptReconnectInterval time.Duration
 }

exporters/jaeger/agent_test.go

@@ -16,7 +16,6 @@ package jaeger
 
 import (
 	"context"
-	"log"
 	"net"
 	"testing"
 
@@ -54,7 +53,7 @@ func TestNewAgentClientUDPWithParams(t *testing.T) {
 	assert.Equal(t, 25000, agentClient.maxPacketSize)
 
 	if assert.IsType(t, &reconnectingUDPConn{}, agentClient.connUDP) {
-		assert.Equal(t, (*log.Logger)(nil), agentClient.connUDP.(*reconnectingUDPConn).logger)
+		assert.Equal(t, emptyLogger, agentClient.connUDP.(*reconnectingUDPConn).logger)
 	}
 
 	assert.NoError(t, agentClient.Close())
@@ -77,7 +76,7 @@ func TestNewAgentClientUDPWithParamsDefaults(t *testing.T) {
 	assert.Equal(t, udpPacketMaxLength, agentClient.maxPacketSize)
 
 	if assert.IsType(t, &reconnectingUDPConn{}, agentClient.connUDP) {
-		assert.Equal(t, (*log.Logger)(nil), agentClient.connUDP.(*reconnectingUDPConn).logger)
+		assert.Equal(t, emptyLogger, agentClient.connUDP.(*reconnectingUDPConn).logger)
 	}
 
 	assert.NoError(t, agentClient.Close())
@@ -93,7 +92,7 @@ func TestNewAgentClientUDPWithParamsReconnectingDisabled(t *testing.T) {
 	agentClient, err := newAgentClientUDP(agentClientUDPParams{
 		Host:                host,
 		Port:                port,
-		Logger:              nil,
+		Logger:              emptyLogger,
 		AttemptReconnecting: false,
 	})
 	assert.NoError(t, err)

exporters/jaeger/go.mod

@@ -3,6 +3,8 @@ module go.opentelemetry.io/otel/exporters/jaeger
 go 1.18
 
 require (
+	github.com/go-logr/logr v1.2.3
+	github.com/go-logr/stdr v1.2.2
 	github.com/google/go-cmp v0.5.9
 	github.com/stretchr/testify v1.8.1
 	go.opentelemetry.io/otel v1.11.2
@@ -12,8 +14,6 @@ require (
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/go-logr/logr v1.2.3 // indirect
-	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/stretchr/objx v0.5.0 // indirect
 	golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 // indirect

exporters/jaeger/reconnecting_udp_client.go

@@ -16,11 +16,12 @@ package jaeger // import "go.opentelemetry.io/otel/exporters/jaeger"
 
 import (
 	"fmt"
-	"log"
 	"net"
 	"sync"
 	"sync/atomic"
 	"time"
+
+	"github.com/go-logr/logr"
 )
 
 // reconnectingUDPConn is an implementation of udpConn that resolves hostPort every resolveTimeout, if the resolved address is
@@ -32,7 +33,7 @@ type reconnectingUDPConn struct {
 	hostPort    string
 	resolveFunc resolveFunc
 	dialFunc    dialFunc
-	logger      *log.Logger
+	logger      logr.Logger
 
 	connMtx   sync.RWMutex
 	conn      *net.UDPConn
@@ -45,7 +46,7 @@ type dialFunc func(network string, laddr, raddr *net.UDPAddr) (*net.UDPConn, err
 
 // newReconnectingUDPConn returns a new udpConn that resolves hostPort every resolveTimeout, if the resolved address is
 // different than the current conn then the new address is dialed and the conn is swapped.
-func newReconnectingUDPConn(hostPort string, bufferBytes int, resolveTimeout time.Duration, resolveFunc resolveFunc, dialFunc dialFunc, logger *log.Logger) (*reconnectingUDPConn, error) {
+func newReconnectingUDPConn(hostPort string, bufferBytes int, resolveTimeout time.Duration, resolveFunc resolveFunc, dialFunc dialFunc, logger logr.Logger) (*reconnectingUDPConn, error) {
 	conn := &reconnectingUDPConn{
 		hostPort:    hostPort,
 		resolveFunc: resolveFunc,
@@ -65,8 +66,8 @@ func newReconnectingUDPConn(hostPort string, bufferBytes int, resolveTimeout tim
 }
 
 func (c *reconnectingUDPConn) logf(format string, args ...interface{}) {
-	if c.logger != nil {
-		c.logger.Printf(format, args...)
+	if c.logger != emptyLogger {
+		c.logger.Info(format, args...)
 	}
 }
 

exporters/jaeger/reconnecting_udp_client_test.go

@@ -88,7 +88,7 @@ func assertConnWritable(t *testing.T, conn udpConn, serverConn net.PacketConn) {
 	_, err := conn.Write([]byte(expectedString))
 	require.NoError(t, err)
 
-	var buf = make([]byte, len(expectedString))
+	buf := make([]byte, len(expectedString))
 	err = serverConn.SetReadDeadline(time.Now().Add(time.Second))
 	require.NoError(t, err)
 
@@ -145,7 +145,7 @@ func waitForConnCondition(conn *reconnectingUDPConn, condition func(conn *reconn
 }
 
 func newMockUDPAddr(t *testing.T, port int) *net.UDPAddr {
-	var buf = make([]byte, 4)
+	buf := make([]byte, 4)
 	// random is not seeded to ensure tests are deterministic (also doesnt matter if ip is valid)
 	_, err := rand.Read(buf)
 	require.NoError(t, err)
@@ -177,7 +177,7 @@ func TestNewResolvedUDPConn(t *testing.T) {
 		Return(clientConn, nil).
 		Once()
 
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Hour, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Hour, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 
@@ -212,7 +212,7 @@ func TestResolvedUDPConnWrites(t *testing.T) {
 		Return(clientConn, nil).
 		Once()
 
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Hour, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Hour, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 
@@ -249,7 +249,7 @@ func TestResolvedUDPConnEventuallyDials(t *testing.T) {
 		On("DialUDP", "udp", (*net.UDPAddr)(nil), mockUDPAddr).
 		Return(clientConn, nil).Once()
 
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 
@@ -300,7 +300,7 @@ func TestResolvedUDPConnNoSwapIfFail(t *testing.T) {
 		On("DialUDP", "udp", (*net.UDPAddr)(nil), mockUDPAddr).
 		Return(clientConn, nil).Once()
 
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 
@@ -341,7 +341,7 @@ func TestResolvedUDPConnWriteRetry(t *testing.T) {
 		On("DialUDP", "udp", (*net.UDPAddr)(nil), mockUDPAddr).
 		Return(clientConn, nil).Once()
 
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 
@@ -371,7 +371,7 @@ func TestResolvedUDPConnWriteRetryFails(t *testing.T) {
 
 	dialer := mockDialer{}
 
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 
@@ -428,7 +428,7 @@ func TestResolvedUDPConnChanges(t *testing.T) {
 		On("DialUDP", "udp", (*net.UDPAddr)(nil), mockUDPAddr2).
 		Return(clientConn2, nil).Once()
 
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, time.Millisecond*10, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 
@@ -481,7 +481,7 @@ func TestResolvedUDPConnLoopWithoutChanges(t *testing.T) {
 		Once()
 
 	resolveTimeout := 500 * time.Millisecond
-	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, resolveTimeout, resolver.ResolveUDPAddr, dialer.DialUDP, nil)
+	conn, err := newReconnectingUDPConn(hostPort, udpPacketMaxLength, resolveTimeout, resolver.ResolveUDPAddr, dialer.DialUDP, emptyLogger)
 	assert.NoError(t, err)
 	require.NotNil(t, conn)
 	assert.Equal(t, mockUDPAddr, conn.destAddr)

exporters/jaeger/uploader.go

@@ -23,6 +23,9 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/go-logr/logr"
+	"github.com/go-logr/stdr"
+
 	gen "go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger"
 	"go.opentelemetry.io/otel/exporters/jaeger/internal/third_party/thrift/lib/go/thrift"
 )
@@ -112,8 +115,17 @@ func WithAgentPort(port string) AgentEndpointOption {
 	})
 }
 
+var emptyLogger = logr.Logger{}
+
 // WithLogger sets a logger to be used by agent client.
+// WithLogger and WithLogr will overwrite each other.
 func WithLogger(logger *log.Logger) AgentEndpointOption {
+	return WithLogr(stdr.New(logger))
+}
+
+// WithLogr sets a logr.Logger to be used by agent client.
+// WithLogr and WithLogger will overwrite each other.
+func WithLogr(logger logr.Logger) AgentEndpointOption {
 	return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
 		o.Logger = logger
 		return o

exporters/zipkin/go.mod

@@ -3,6 +3,8 @@ module go.opentelemetry.io/otel/exporters/zipkin
 go 1.18
 
 require (
+	github.com/go-logr/logr v1.2.3
+	github.com/go-logr/stdr v1.2.2
 	github.com/google/go-cmp v0.5.9
 	github.com/openzipkin/zipkin-go v0.4.1
 	github.com/stretchr/testify v1.8.1
@@ -13,8 +15,6 @@ require (
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/go-logr/logr v1.2.3 // indirect
-	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	golang.org/x/sys v0.0.0-20221010170243-090e33056c14 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

exporters/zipkin/zipkin.go

@@ -25,6 +25,9 @@ import (
 	"net/url"
 	"sync"
 
+	"github.com/go-logr/logr"
+	"github.com/go-logr/stdr"
+
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 )
 
@@ -36,20 +39,20 @@ const (
 type Exporter struct {
 	url    string
 	client *http.Client
-	logger *log.Logger
+	logger logr.Logger
 
 	stoppedMu sync.RWMutex
 	stopped   bool
 }
 
-var (
-	_ sdktrace.SpanExporter = &Exporter{}
-)
+var _ sdktrace.SpanExporter = &Exporter{}
+
+var emptyLogger = logr.Logger{}
 
 // Options contains configuration for the exporter.
 type config struct {
 	client *http.Client
-	logger *log.Logger
+	logger logr.Logger
 }
 
 // Option defines a function that configures the exporter.
@@ -64,7 +67,14 @@ func (fn optionFunc) apply(cfg config) config {
 }
 
 // WithLogger configures the exporter to use the passed logger.
+// WithLogger and WithLogr will overwrite each other.
 func WithLogger(logger *log.Logger) Option {
+	return WithLogr(stdr.New(logger))
+}
+
+// WithLogr configures the exporter to use the passed logr.Logger.
+// WithLogr and WithLogger will overwrite each other.
+func WithLogr(logger logr.Logger) Option {
 	return optionFunc(func(cfg config) config {
 		cfg.logger = logger
 		return cfg
@@ -170,8 +180,8 @@ func (e *Exporter) Shutdown(ctx context.Context) error {
 }
 
 func (e *Exporter) logf(format string, args ...interface{}) {
-	if e.logger != nil {
-		e.logger.Printf(format, args...)
+	if e.logger != emptyLogger {
+		e.logger.Info(format, args)
 	}
 }