diff --git a/CHANGELOG.md b/CHANGELOG.md index 0486e8de8d8..6cb2b592746 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/exporters/jaeger/agent.go b/exporters/jaeger/agent.go index ed002b83f7a..a050020bb47 100644 --- a/exporters/jaeger/agent.go +++ b/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 } diff --git a/exporters/jaeger/agent_test.go b/exporters/jaeger/agent_test.go index fdcd9bb74d6..95070341722 100644 --- a/exporters/jaeger/agent_test.go +++ b/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) diff --git a/exporters/jaeger/go.mod b/exporters/jaeger/go.mod index 0ee4a1cb2bc..c1d38fabe1f 100644 --- a/exporters/jaeger/go.mod +++ b/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 diff --git a/exporters/jaeger/reconnecting_udp_client.go b/exporters/jaeger/reconnecting_udp_client.go index fa2c68ead42..88055c8a300 100644 --- a/exporters/jaeger/reconnecting_udp_client.go +++ b/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...) } } diff --git a/exporters/jaeger/reconnecting_udp_client_test.go b/exporters/jaeger/reconnecting_udp_client_test.go index 43bfe512493..958f6400baf 100644 --- a/exporters/jaeger/reconnecting_udp_client_test.go +++ b/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) diff --git a/exporters/jaeger/uploader.go b/exporters/jaeger/uploader.go index 1bdb4de4f92..f65e3a6782a 100644 --- a/exporters/jaeger/uploader.go +++ b/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 diff --git a/exporters/zipkin/go.mod b/exporters/zipkin/go.mod index b68d7ff3f1e..3cc8e8aa89c 100644 --- a/exporters/zipkin/go.mod +++ b/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 diff --git a/exporters/zipkin/zipkin.go b/exporters/zipkin/zipkin.go index 93b027a1557..b21a0190c2d 100644 --- a/exporters/zipkin/zipkin.go +++ b/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) } }