Skip to content
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

detect: refactor the detect package #4481

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions cmd/buildctl/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"time"

"github.com/moby/buildkit/client"
"github.com/moby/buildkit/util/tracing/detect"
"github.com/moby/buildkit/util/tracing/delegated"
"github.com/pkg/errors"
"github.com/urfave/cli"
"go.opentelemetry.io/otel/trace"
Expand Down Expand Up @@ -68,16 +68,10 @@ func ResolveClient(c *cli.Context) (*client.Client, error) {
ctx := CommandContext(c)
var opts []client.ClientOpt
if span := trace.SpanFromContext(ctx); span.SpanContext().IsValid() {
opts = append(opts, client.WithTracerProvider(span.TracerProvider()))

exp, _, err := detect.Exporter()
if err != nil {
return nil, err
}

if td, ok := exp.(client.TracerDelegate); ok {
opts = append(opts, client.WithTracerDelegate(td))
}
opts = append(opts,
client.WithTracerProvider(span.TracerProvider()),
client.WithTracerDelegate(delegated.DefaultExporter),
)
}

if caCert != "" {
Expand Down
12 changes: 10 additions & 2 deletions cmd/buildctl/common/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,28 @@ import (
"os"

"github.com/moby/buildkit/util/appcontext"
"github.com/moby/buildkit/util/tracing/delegated"
"github.com/moby/buildkit/util/tracing/detect"
"github.com/urfave/cli"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/trace"
)

func AttachAppContext(app *cli.App) error {
ctx := appcontext.Context()

tp, err := detect.TracerProvider()
exp, err := detect.NewSpanExporter(ctx)
if err != nil {
return err
}

tp := sdktrace.NewTracerProvider(
sdktrace.WithResource(detect.Resource()),
sdktrace.WithBatcher(exp),
sdktrace.WithBatcher(delegated.DefaultExporter),
)
tracer := tp.Tracer("")

var span trace.Span
Expand Down Expand Up @@ -60,7 +68,7 @@ func AttachAppContext(app *cli.App) error {
if span != nil {
span.End()
}
return detect.Shutdown(context.TODO())
return tp.Shutdown(context.TODO())
}
return nil
}
Expand Down
1 change: 0 additions & 1 deletion cmd/buildctl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/moby/buildkit/util/appdefaults"
"github.com/moby/buildkit/util/profiler"
"github.com/moby/buildkit/util/stack"
_ "github.com/moby/buildkit/util/tracing/detect/delegated"
_ "github.com/moby/buildkit/util/tracing/detect/jaeger"
_ "github.com/moby/buildkit/util/tracing/env"
"github.com/moby/buildkit/version"
Expand Down
71 changes: 62 additions & 9 deletions cmd/buildkitd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
sddaemon "github.com/coreos/go-systemd/v22/daemon"
"github.com/docker/docker/pkg/reexec"
"github.com/gofrs/flock"
"github.com/hashicorp/go-multierror"
"github.com/moby/buildkit/cache/remotecache"
"github.com/moby/buildkit/cache/remotecache/azblob"
"github.com/moby/buildkit/cache/remotecache/gha"
Expand Down Expand Up @@ -63,7 +64,9 @@ import (
"github.com/urfave/cli"
"go.etcd.io/bbolt"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"go.opentelemetry.io/otel/exporters/prometheus"
"go.opentelemetry.io/otel/propagation"
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
tracev1 "go.opentelemetry.io/proto/otlp/collector/trace/v1"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -217,6 +220,7 @@ func main() {
app.Flags = append(app.Flags, appFlags...)
app.Flags = append(app.Flags, serviceFlags()...)

var closers []func(ctx context.Context) error
app.Action = func(c *cli.Context) error {
// TODO: On Windows this always returns -1. The actual "are you admin" check is very Windows-specific.
// See https://github.com/golang/go/issues/28804#issuecomment-505326268 for the "short" version.
Expand Down Expand Up @@ -259,15 +263,17 @@ func main() {
}
}

tp, err := detect.TracerProvider()
tp, err := newTracerProvider(ctx)
if err != nil {
return err
}
closers = append(closers, tp.Shutdown)

mp, err := detect.MeterProvider()
mp, err := newMeterProvider(ctx)
if err != nil {
return err
}
closers = append(closers, mp.Shutdown)

statsHandler := tracing.ServerStatsHandler(
otelgrpc.WithTracerProvider(tp),
Expand Down Expand Up @@ -375,8 +381,13 @@ func main() {
return err
}

app.After = func(_ *cli.Context) error {
return detect.Shutdown(context.TODO())
app.After = func(_ *cli.Context) (err error) {
for _, c := range closers {
if e := c(context.TODO()); e != nil {
err = multierror.Append(err, e)
}
}
return err
}

profiler.Attach(app)
Expand Down Expand Up @@ -737,13 +748,19 @@ func newController(c *cli.Context, cfg *config.Config) (*control.Controller, err
return nil, err
}

tc, _, err := detect.Exporter()
if err != nil {
tc := make(tracing.MultiSpanExporter, 0, 2)
if detect.Recorder != nil {
tc = append(tc, detect.Recorder)
}

if exp, err := detect.NewSpanExporter(context.TODO()); err != nil {
return nil, err
} else if !detect.IsNoneSpanExporter(exp) {
tc = append(tc, exp)
}

var traceSocket string
if tc != nil {
if len(tc) > 0 {
traceSocket = cfg.OTEL.SocketPath
if err := runTraceController(traceSocket, tc); err != nil {
return nil, err
Expand Down Expand Up @@ -942,9 +959,45 @@ type traceCollector struct {
}

func (t *traceCollector) Export(ctx context.Context, req *tracev1.ExportTraceServiceRequest) (*tracev1.ExportTraceServiceResponse, error) {
err := t.exporter.ExportSpans(ctx, transform.Spans(req.GetResourceSpans()))
if err != nil {
if err := t.exporter.ExportSpans(ctx, transform.Spans(req.GetResourceSpans())); err != nil {
return nil, err
}
return &tracev1.ExportTraceServiceResponse{}, nil
}

func newTracerProvider(ctx context.Context) (*sdktrace.TracerProvider, error) {
opts := []sdktrace.TracerProviderOption{
sdktrace.WithResource(detect.Resource()),
sdktrace.WithSyncer(detect.Recorder),
}

if exp, err := detect.NewSpanExporter(ctx); err != nil {
return nil, err
} else if !detect.IsNoneSpanExporter(exp) {
opts = append(opts, sdktrace.WithBatcher(exp))
}
return sdktrace.NewTracerProvider(opts...), nil
}

func newMeterProvider(ctx context.Context) (*sdkmetric.MeterProvider, error) {
opts := []sdkmetric.Option{
sdkmetric.WithResource(detect.Resource()),
}

if r, err := prometheus.New(); err != nil {
// Log the error but do not fail if we could not configure the prometheus metrics.
bklog.G(context.Background()).
WithError(err).
Error("failed prometheus metrics configuration")
} else {
opts = append(opts, sdkmetric.WithReader(r))
}

if exp, err := detect.NewMetricExporter(ctx); err != nil {
return nil, err
} else if !detect.IsNoneMetricExporter(exp) {
r := sdkmetric.NewPeriodicReader(exp)
opts = append(opts, sdkmetric.WithReader(r))
}
return sdkmetric.NewMeterProvider(opts...), nil
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ require (
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.21.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.21.0
go.opentelemetry.io/otel/exporters/prometheus v0.42.0
go.opentelemetry.io/otel/metric v1.21.0
go.opentelemetry.io/otel/sdk v1.21.0
go.opentelemetry.io/otel/sdk/metric v1.21.0
go.opentelemetry.io/otel/trace v1.21.0
Expand Down Expand Up @@ -165,6 +164,7 @@ require (
github.com/vishvananda/netns v0.0.4 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0 // indirect
go.opentelemetry.io/otel/metric v1.21.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/tools v0.14.0 // indirect
google.golang.org/genproto v0.0.0-20231016165738-49dd2c1f3d0b // indirect
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,24 @@ import (
"context"
"sync"

"github.com/moby/buildkit/util/tracing/detect"
"github.com/moby/buildkit/client"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
)

const maxBuffer = 256

var exp = &Exporter{}

func init() {
detect.Register("delegated", detect.TraceExporterDetector(func() (sdktrace.SpanExporter, error) {
return exp, nil
}), 100)
}
var DefaultExporter = &Exporter{}

type Exporter struct {
mu sync.Mutex
exporters []sdktrace.SpanExporter
buffer []sdktrace.ReadOnlySpan
}

var _ sdktrace.SpanExporter = &Exporter{}
var (
_ sdktrace.SpanExporter = (*Exporter)(nil)
_ client.TracerDelegate = (*Exporter)(nil)
)

func (e *Exporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error {
e.mu.Lock()
Expand Down
18 changes: 0 additions & 18 deletions util/tracing/detect/delegated/delegated_test.go

This file was deleted.

Loading
Loading