Skip to content

Commit

Permalink
feat(exporter): Jaeger and Zipkin exporter use `github.com/go-logr/lo…
Browse files Browse the repository at this point in the history
…gr` as the logging interface, and add the WithLogr option
  • Loading branch information
aniaan committed Nov 30, 2022
1 parent 289a612 commit eb27827
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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)

### Fixed

Expand Down
4 changes: 2 additions & 2 deletions exporters/jaeger/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ package jaeger // import "go.opentelemetry.io/otel/exporters/jaeger"
import (
"context"
"fmt"
"github.com/go-logr/logr"
"io"
"log"
"net"
"strings"
"time"
Expand Down Expand Up @@ -58,7 +58,7 @@ type agentClientUDPParams struct {
Host string
Port string
MaxPacketSize int
Logger *log.Logger
Logger logr.Logger
AttemptReconnecting bool
AttemptReconnectInterval time.Duration
}
Expand Down
7 changes: 3 additions & 4 deletions exporters/jaeger/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package jaeger

import (
"context"
"log"
"net"
"testing"

Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions exporters/jaeger/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.1
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions exporters/jaeger/reconnecting_udp_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ package jaeger // import "go.opentelemetry.io/otel/exporters/jaeger"

import (
"fmt"
"log"
"github.com/go-logr/logr"
"net"
"sync"
"sync/atomic"
Expand All @@ -32,7 +32,7 @@ type reconnectingUDPConn struct {
hostPort string
resolveFunc resolveFunc
dialFunc dialFunc
logger *log.Logger
logger logr.Logger

connMtx sync.RWMutex
conn *net.UDPConn
Expand All @@ -45,7 +45,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,
Expand All @@ -65,8 +65,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...)
}
}

Expand Down
16 changes: 8 additions & 8 deletions exporters/jaeger/reconnecting_udp_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions exporters/jaeger/uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"bytes"
"context"
"fmt"
"github.com/go-logr/logr"
"github.com/go-logr/stdr"
"io"
"log"
"net/http"
Expand Down Expand Up @@ -112,8 +114,18 @@ func WithAgentPort(port string) AgentEndpointOption {
})
}

var emptyLogger = logr.Logger{}

// WithLogger sets a logger to be used by agent client.
func WithLogger(logger *log.Logger) AgentEndpointOption {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.Logger = stdr.New(logger)
return o
})
}

// WithLogr sets a logr.Logger to be used by agent client.
func WithLogr(logger logr.Logger) AgentEndpointOption {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.Logger = logger
return o
Expand Down
4 changes: 2 additions & 2 deletions exporters/zipkin/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
21 changes: 16 additions & 5 deletions exporters/zipkin/zipkin.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/go-logr/logr"
"github.com/go-logr/stdr"
"io"
"log"
"net/http"
Expand All @@ -36,20 +38,21 @@ const (
type Exporter struct {
url string
client *http.Client
logger *log.Logger
logger logr.Logger

stoppedMu sync.RWMutex
stopped bool
}

var (
_ sdktrace.SpanExporter = &Exporter{}
_ sdktrace.SpanExporter = &Exporter{}
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.
Expand All @@ -65,6 +68,14 @@ func (fn optionFunc) apply(cfg config) config {

// WithLogger configures the exporter to use the passed logger.
func WithLogger(logger *log.Logger) Option {
return optionFunc(func(cfg config) config {
cfg.logger = stdr.New(logger)
return cfg
})
}

// WithLogr configures the exporter to use the passed logr.Logger.
func WithLogr(logger logr.Logger) Option {
return optionFunc(func(cfg config) config {
cfg.logger = logger
return cfg
Expand Down Expand Up @@ -170,8 +181,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)
}
}

Expand Down

0 comments on commit eb27827

Please sign in to comment.