-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instrument Otel #125
Merged
Merged
Instrument Otel #125
Changes from 24 commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
cbdda80
initial tracing try with background.get
louiseschmidtgen ecda7a8
more updates backend.get
louiseschmidtgen 6b95d6c
parent spans
louiseschmidtgen 38e3b21
logs to jaeger
louiseschmidtgen 8514849
svc name
louiseschmidtgen 0be0a10
traces mostly list
louiseschmidtgen 87e5548
instrument create
louiseschmidtgen 4e0dd87
instrument delete and update
louiseschmidtgen 089c4d3
dbsize
louiseschmidtgen 3be9a84
cleanup
louiseschmidtgen 478cbb7
cleanup 2
louiseschmidtgen 788fcf3
Merge remote-tracking branch 'origin' into KU-858/tracing
louiseschmidtgen 755e113
cleanup 3
louiseschmidtgen 10ac5db
add service name to meterprovider
louiseschmidtgen 6913487
cleanup
louiseschmidtgen 778a82c
comments
louiseschmidtgen 67dc71f
move otel setup
louiseschmidtgen 988deca
cleanup
louiseschmidtgen 213b745
remove wrong place compact logs
louiseschmidtgen 85af5c3
add traces to compaction
louiseschmidtgen 0534bef
on update's create if errkeyexists do a get
louiseschmidtgen 81ef2bb
trace names
louiseschmidtgen 9143cc9
ctx
louiseschmidtgen 2681a0e
ctx
louiseschmidtgen 38c31b3
move run jaeger instance to script
louiseschmidtgen 0febbb9
comments part 1
louiseschmidtgen f56eb46
move counters closer to where they are used
louiseschmidtgen 2733d4e
traces in logstructured
louiseschmidtgen ceb9c8f
capture more logs on compaction
louiseschmidtgen ada277b
cleanup
louiseschmidtgen cda87ce
naming
louiseschmidtgen 8577851
pass conn address
louiseschmidtgen 0bb3126
cleanup
louiseschmidtgen 04ab821
rewrite otel setup, disable otel by default
louiseschmidtgen f081376
move otel shutdown
louiseschmidtgen b42ceea
typo
louiseschmidtgen bd4d75f
counter init and naming
louiseschmidtgen fcdae60
more renaming
louiseschmidtgen da1622a
comments
louiseschmidtgen 90a04ec
shutdown conn
louiseschmidtgen 2b1887c
handling errs
louiseschmidtgen 7dbfb74
adjust ignore compacted
louiseschmidtgen b9c235e
spaces
louiseschmidtgen 9023e1e
tidy
louiseschmidtgen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
package cmd | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
"fmt" | ||
"time" | ||
|
||
"go.opentelemetry.io/otel" | ||
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" | ||
"go.opentelemetry.io/otel/exporters/stdout/stdoutmetric" | ||
"go.opentelemetry.io/otel/sdk/metric" | ||
"go.opentelemetry.io/otel/sdk/resource" | ||
"go.opentelemetry.io/otel/sdk/trace" | ||
semconv "go.opentelemetry.io/otel/semconv/v1.4.0" | ||
"google.golang.org/grpc" | ||
"google.golang.org/grpc/credentials/insecure" | ||
) | ||
|
||
// setupOTelSDK bootstraps the OpenTelemetry pipeline. | ||
// If it does not return an error, make sure to call shutdown for proper cleanup. | ||
func setupOTelSDK(ctx context.Context) (shutdown func(context.Context) error, err error) { | ||
var shutdownFuncs []func(context.Context) error | ||
|
||
shutdown = func(ctx context.Context) error { | ||
var err error | ||
for _, fn := range shutdownFuncs { | ||
err = errors.Join(err, fn(ctx)) | ||
} | ||
shutdownFuncs = nil | ||
return err | ||
} | ||
|
||
handleErr := func(inErr error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be handled in a defer instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've removed it alltogether. |
||
err = errors.Join(inErr, shutdown(ctx)) | ||
} | ||
|
||
res, err := resource.New(ctx, | ||
marco6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
resource.WithAttributes( | ||
semconv.ServiceNameKey.String("k8s-dqlite"), | ||
), | ||
) | ||
|
||
tracerProvider, err := newTraceProvider(ctx, res) | ||
if err != nil { | ||
handleErr(err) | ||
return | ||
} | ||
shutdownFuncs = append(shutdownFuncs, tracerProvider.Shutdown) | ||
marco6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
otel.SetTracerProvider(tracerProvider) | ||
|
||
meterProvider, err := newMeterProvider(res) | ||
if err != nil { | ||
handleErr(err) | ||
return | ||
} | ||
shutdownFuncs = append(shutdownFuncs, meterProvider.Shutdown) | ||
otel.SetMeterProvider(meterProvider) | ||
|
||
return | ||
} | ||
|
||
func initConn() (*grpc.ClientConn, error) { | ||
// It connects the OpenTelemetry Collector through local gRPC connection. | ||
// You may replace `localhost:4317` with your endpoint. | ||
conn, err := grpc.NewClient("localhost:4317", | ||
louiseschmidtgen marked this conversation as resolved.
Show resolved
Hide resolved
louiseschmidtgen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Note the use of insecure transport here. TLS is recommended in production. | ||
grpc.WithTransportCredentials(insecure.NewCredentials()), | ||
) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to create gRPC connection to collector: %w", err) | ||
} | ||
|
||
return conn, err | ||
louiseschmidtgen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
func newExporter(ctx context.Context) (trace.SpanExporter, error) { | ||
louiseschmidtgen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
conn, _ := initConn() | ||
|
||
traceExporter, err := otlptracegrpc.New(ctx, otlptracegrpc.WithGRPCConn(conn)) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to create trace exporter: %w", err) | ||
} | ||
return traceExporter, nil | ||
} | ||
|
||
func newTraceProvider(ctx context.Context, res *resource.Resource) (*trace.TracerProvider, error) { | ||
traceExporter, err := newExporter(ctx) | ||
marco6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
traceProvider := trace.NewTracerProvider( | ||
trace.WithBatcher(traceExporter, | ||
trace.WithBatchTimeout(time.Second), | ||
), | ||
trace.WithResource(res), | ||
) | ||
return traceProvider, nil | ||
} | ||
|
||
func newMeterProvider(res *resource.Resource) (*metric.MeterProvider, error) { | ||
metricExporter, err := stdoutmetric.New() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
meterProvider := metric.NewMeterProvider( | ||
metric.WithReader(metric.NewPeriodicReader(metricExporter, | ||
metric.WithInterval(30*time.Second))), | ||
metric.WithResource(res), | ||
) | ||
return meterProvider, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern seems a bit obscure IMHO. The logic is opaque as it's just a list of functions, but maybe we should instead have it explicit? Consider that having a list of functions allows for a dynamic number of cleanup logic, but in this function the number of objects to close is statically known.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True this makes sense if you collect logs etc here too.