Skip to content

Commit

Permalink
fix up otel init (#73)
Browse files Browse the repository at this point in the history 
Otel initialization is still somewhat hacky, but we're moving towards a
cleaner implementation This change introduces the metrics (aka, meter)
producer, and moves all producers (trace, log, metrics) into their own
constructor funcs.

Next PR will use this provider to implement a metrics api.
  • Loading branch information
@ryanfkeepers
ryanfkeepers authored Dec 3, 2024
1 parent 36665d8 commit 078ed21
Show file tree
Hide file tree
Showing 7 changed files with 264 additions and 86 deletions.
6 changes: 4 additions & 2 deletions clog/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ func (b builder) log(l logLevel, msg string) {
// add otel logging if provided
otelLogger := b.otel

if otelLogger == nil {
otelLogger = cluesNode.OTELLogger()
if otelLogger == nil &&
cluesNode.OTEL != nil &&
cluesNode.OTEL.Logger != nil {
otelLogger = cluesNode.OTEL.Logger
}

if otelLogger != nil {
Expand Down
17 changes: 10 additions & 7 deletions clog/logger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"sync"
"time"

otellog "go.opentelemetry.io/otel/log"
"go.opentelemetry.io/otel/log"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"

Expand All @@ -23,7 +23,7 @@ var (
)

type clogger struct {
otel otellog.Logger
otel log.Logger
zsl *zap.SugaredLogger
set Settings
}
Expand Down Expand Up @@ -146,12 +146,15 @@ func singleton(ctx context.Context, set Settings) *clogger {

zsl := genLogger(set)

otelLogger := clues.In(ctx).OTELLogger()

cloggerton = &clogger{
otel: otelLogger,
zsl: zsl,
set: set,
zsl: zsl,
set: set,
}

node := clues.In(ctx)

if node.OTEL != nil && node.OTEL.Logger != nil {
cloggerton.otel = node.OTEL.Logger
}

return cloggerton
Expand Down
25 changes: 16 additions & 9 deletions clues_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"regexp"
"testing"

"github.com/stretchr/testify/require"

"github.com/alcionai/clues"
"github.com/alcionai/clues/cecrets"
"github.com/alcionai/clues/internal/tester"
Expand Down Expand Up @@ -104,25 +106,30 @@ func TestAddSpan(t *testing.T) {
}
for _, test := range table {
for _, init := range []bool{true, false} {
t.Run(test.name, func(t *testing.T) {
tname := fmt.Sprintf("%s-%v", test.name, init)

t.Run(tname, func(t *testing.T) {
ctx := context.Background()

if init {
ocfg := clues.OTELConfig{GRPCEndpoint: "localhost:4317"}

ictx, err := clues.InitializeOTEL(ctx, test.name, ocfg)
require.NoError(t, err, "initializing otel")
if err != nil {
t.Error("initializing clues", err)
return
}

defer func() {
err := clues.Close(ictx)
if err != nil {
t.Error("closing clues:", err)
return
}
}()
// FIXME: this is causing failures at the moment which are non-trivial to
// hack around. Will need to return to it for more complete otel/grpc testing.
// suggestion: https://github.com/pellared/opentelemetry-go-contrib/blob/8f8e9b60693177b91af45d0495289fc52aa5c50e/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_test.go#L88
// defer func() {
// err := clues.Close(ictx)
// require.NoError(t, err, "closing clues")
// if err != nil {
// return
// }
// }()

ctx = ictx
}
Expand Down
6 changes: 5 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@ require (
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/otel v1.31.0
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.7.0
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.31.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.31.0
go.opentelemetry.io/otel/log v0.7.0
go.opentelemetry.io/otel/metric v1.31.0
go.opentelemetry.io/otel/sdk v1.31.0
go.opentelemetry.io/otel/sdk/log v0.7.0
go.opentelemetry.io/otel/sdk/metric v1.31.0
go.opentelemetry.io/otel/trace v1.31.0
go.uber.org/zap v1.27.0
golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f
Expand All @@ -24,7 +29,6 @@ require (
github.com/grpc-ecosystem/grpc-gateway/v2 v2.22.0 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.31.0 // indirect
go.opentelemetry.io/otel/metric v1.31.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/net v0.30.0 // indirect
Expand Down
8 changes: 8 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
go.opentelemetry.io/otel v1.31.0 h1:NsJcKPIW0D0H3NgzPDHmo0WW6SptzPdqg/L1zsIm2hY=
go.opentelemetry.io/otel v1.31.0/go.mod h1:O0C14Yl9FgkjqcCZAsE053C13OaddMYr/hz6clDkEJE=
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.7.0 h1:iNba3cIZTDPB2+IAbVY/3TUN+pCCLrNYo2GaGtsKBak=
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.7.0/go.mod h1:l5BDPiZ9FbeejzWTAX6BowMzQOM/GeaUQ6lr3sOcSkc=
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.31.0 h1:FZ6ei8GFW7kyPYdxJaV2rgI6M+4tvZzhYsQ2wgyVC08=
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.31.0/go.mod h1:MdEu/mC6j3D+tTEfvI15b5Ci2Fn7NneJ71YMoiS3tpI=
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.31.0 h1:K0XaT3DwHAcV4nKLzcQvwAgSyisUghWoY20I7huthMk=
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.31.0/go.mod h1:B5Ki776z/MBnVha1Nzwp5arlzBbE3+1jk+pGmaP5HME=
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.31.0 h1:FFeLy03iVTXP6ffeN2iXrxfGsZGCjVx0/4KlizjyBwU=
Expand All @@ -37,6 +41,10 @@ go.opentelemetry.io/otel/metric v1.31.0 h1:FSErL0ATQAmYHUIzSezZibnyVlft1ybhy4ozR
go.opentelemetry.io/otel/metric v1.31.0/go.mod h1:C3dEloVbLuYoX41KpmAhOqNriGbA+qqH6PQ5E5mUfnY=
go.opentelemetry.io/otel/sdk v1.31.0 h1:xLY3abVHYZ5HSfOg3l2E5LUj2Cwva5Y7yGxnSW9H5Gk=
go.opentelemetry.io/otel/sdk v1.31.0/go.mod h1:TfRbMdhvxIIr/B2N2LQW2S5v9m3gOQ/08KsbbO5BPT0=
go.opentelemetry.io/otel/sdk/log v0.7.0 h1:dXkeI2S0MLc5g0/AwxTZv6EUEjctiH8aG14Am56NTmQ=
go.opentelemetry.io/otel/sdk/log v0.7.0/go.mod h1:oIRXpW+WD6M8BuGj5rtS0aRu/86cbDV/dAfNaZBIjYM=
go.opentelemetry.io/otel/sdk/metric v1.31.0 h1:i9hxxLJF/9kkvfHppyLL55aW7iIJz4JjxTeYusH7zMc=
go.opentelemetry.io/otel/sdk/metric v1.31.0/go.mod h1:CRInTMVvNhUKgSAMbKyTMxqOBC0zgyxzW55lZzX43Y8=
go.opentelemetry.io/otel/trace v1.31.0 h1:ffjsj1aRouKewfr85U2aGagJ46+MvodynlQ1HYdmJys=
go.opentelemetry.io/otel/trace v1.31.0/go.mod h1:TXZkRk7SM2ZQLtR6eoAWQFIHPvzQ06FJAsO1tJg480A=
go.opentelemetry.io/proto/otlp v1.3.1 h1:TrMUixzpM0yuc/znrFTP9MMRh8trP93mkCiDVeXrui0=
Expand Down
30 changes: 17 additions & 13 deletions internal/node/node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,35 @@ import (

func TestNode_Init(t *testing.T) {
table := []struct {
name string
node *Node
ctx context.Context
name string
node *Node
ctx context.Context
wantErr require.ErrorAssertionFunc
}{
{
name: "nil ctx",
node: &Node{},
ctx: nil,
name: "nil ctx",
node: &Node{},
ctx: nil,
wantErr: require.Error,
},
{
name: "nil node",
node: nil,
ctx: context.Background(),
name: "nil node",
node: nil,
ctx: context.Background(),
wantErr: require.NoError,
},
{
name: "context.Context",
node: &Node{},
ctx: context.Background(),
name: "context.Context",
node: &Node{},
ctx: context.Background(),
wantErr: require.NoError,
},
}

for _, test := range table {
t.Run(test.name, func(t *testing.T) {
err := test.node.InitOTEL(test.ctx, test.name, OTELConfig{})
require.NoError(t, err)
test.wantErr(t, err)
})
}
}
Expand Down
Loading

0 comments on commit 078ed21

Please sign in to comment.