Skip to content
Open
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
9 changes: 6 additions & 3 deletions internal/build/imgsrc/buildpacks_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import (
packclient "github.com/buildpacks/pack/pkg/client"
projectTypes "github.com/buildpacks/pack/pkg/project/types"
"github.com/pkg/errors"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"

"github.com/superfly/flyctl/internal/cmdfmt"
"github.com/superfly/flyctl/internal/metrics"
"github.com/superfly/flyctl/internal/tracing"
"github.com/superfly/flyctl/iostreams"
"github.com/superfly/flyctl/terminal"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
)

type buildpacksBuilder struct{}
Expand Down Expand Up @@ -158,7 +159,9 @@ func (*buildpacksBuilder) Run(ctx context.Context, dockerFactory *dockerClientFa
build.PushStart()
cmdfmt.PrintBegin(streams.ErrOut, "Pushing image to fly")

if err := pushToFly(ctx, docker, streams, opts.Tag); err != nil {
builderType, builderRegion := getBuilderInfo(ctx, dockerFactory)

if err := pushToFly(ctx, docker, streams, opts.Tag, builderType, builderRegion); err != nil {
build.PushFinish()
return nil, "", err
}
Expand Down
8 changes: 5 additions & 3 deletions internal/build/imgsrc/builtin_builder.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package imgsrc

import (
"fmt"

"context"
"fmt"

"github.com/pkg/errors"

"github.com/superfly/flyctl/internal/build/imgsrc/builtins"
"github.com/superfly/flyctl/internal/cmdfmt"
"github.com/superfly/flyctl/internal/metrics"
Expand Down Expand Up @@ -126,7 +126,9 @@ func (*builtinBuilder) Run(ctx context.Context, dockerFactory *dockerClientFacto
build.PushStart()
cmdfmt.PrintBegin(streams.ErrOut, "Pushing image to fly")

if err := pushToFly(ctx, docker, streams, opts.Tag); err != nil {
builderType, builderRegion := getBuilderInfo(ctx, dockerFactory)

if err := pushToFly(ctx, docker, streams, opts.Tag, builderType, builderRegion); err != nil {
build.PushFinish()
return nil, "", err
}
Expand Down
108 changes: 74 additions & 34 deletions internal/build/imgsrc/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import (
"github.com/pkg/errors"
"github.com/spf13/viper"
fly "github.com/superfly/fly-go"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"

"github.com/superfly/flyctl/agent"
"github.com/superfly/flyctl/flyctl"
"github.com/superfly/flyctl/helpers"
Expand All @@ -37,8 +40,6 @@ import (
"github.com/superfly/flyctl/internal/tracing"
"github.com/superfly/flyctl/iostreams"
"github.com/superfly/flyctl/terminal"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
)

var (
Expand All @@ -47,30 +48,39 @@ var (
)

type dockerClientFactory struct {
mode DockerDaemonType
remote bool
buildFn func(ctx context.Context, build *build) (*dockerclient.Client, error)
apiClient flyutil.Client
appName string
mode DockerDaemonType
remote bool
buildFn func(ctx context.Context, build *build) (*dockerclient.Client, error)
apiClient flyutil.Client
appName string
builderRegion string // Store the region for remote builders
}

func newDockerClientFactory(daemonType DockerDaemonType, apiClient flyutil.Client, appName string, streams *iostreams.IOStreams, connectOverWireguard, recreateBuilder bool) *dockerClientFactory {
useManagedBuilder := daemonType.UseManagedBuilder()
remoteFactory := func() *dockerClientFactory {
terminal.Debug("trying remote docker daemon")
return &dockerClientFactory{
mode: daemonType,
remote: true,
buildFn: func(ctx context.Context, build *build) (*dockerclient.Client, error) {
cfg := config.FromContext(ctx)
if cfg.DisableManagedBuilders {
useManagedBuilder = false
}
return newRemoteDockerClient(ctx, apiClient, appName, streams, build, cachedDocker, connectOverWireguard, useManagedBuilder, recreateBuilder)
},
factory := &dockerClientFactory{
mode: daemonType,
remote: true,
apiClient: apiClient,
appName: appName,
}

factory.buildFn = func(ctx context.Context, build *build) (*dockerclient.Client, error) {
cfg := config.FromContext(ctx)
if cfg.DisableManagedBuilders {
useManagedBuilder = false
}
client, region, err := newRemoteDockerClient(ctx, apiClient, appName, streams, build, cachedDocker, connectOverWireguard, useManagedBuilder, recreateBuilder)
if err == nil {
// Store the region in the factory for later use
factory.builderRegion = region
}
return client, err
}

return factory
}

localFactory := func() *dockerClientFactory {
Expand Down Expand Up @@ -236,14 +246,14 @@ func logClearLinesAbove(streams *iostreams.IOStreams, count int) {
}
}

func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appName string, streams *iostreams.IOStreams, build *build, cachedClient *dockerclient.Client, connectOverWireguard, useManagedBuilder bool, recreateBuilder bool) (c *dockerclient.Client, err error) {
func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appName string, streams *iostreams.IOStreams, build *build, cachedClient *dockerclient.Client, connectOverWireguard, useManagedBuilder bool, recreateBuilder bool) (c *dockerclient.Client, region string, err error) {
ctx, span := tracing.GetTracer().Start(ctx, "build_remote_docker_client", trace.WithAttributes(
attribute.Bool("connect_over_wireguard", connectOverWireguard),
))
defer span.End()
if cachedClient != nil {
span.AddEvent("using cached docker client")
return cachedClient, nil
return cachedClient, "", nil
}

startedAt := time.Now()
Expand All @@ -264,7 +274,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
}
if err != nil {
tracing.RecordError(span, err, "failed to init remote builder machine")
return nil, err
return nil, "", err
}

if useManagedBuilder {
Expand All @@ -286,7 +296,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
tracing.RecordError(span, err, "failed to create remote builder request")
return nil, err
return nil, "", err
}

req.SetBasicAuth(appName, config.Tokens(ctx).Docker())
Expand All @@ -297,7 +307,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
res, err := client.Do(req)
if err != nil {
tracing.RecordError(span, err, "failed to get remote builder settings")
return nil, err
return nil, "", err
}

if res.StatusCode == http.StatusNotFound {
Expand All @@ -308,14 +318,14 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
err := apiClient.DeleteApp(ctx, app.Name)
if err != nil {
tracing.RecordError(span, err, "failed to destroy old incompatible remote builder")
return nil, err
return nil, "", err
}

fmt.Fprintln(streams.Out, streams.ColorScheme().Yellow("🔧 creating fresh remote builder, (this might take a while ...)"))
machine, app, err = remoteBuilderMachine(ctx, apiClient, appName, false)
if err != nil {
tracing.RecordError(span, err, "failed to init remote builder machine")
return nil, err
return nil, "", err
}
logClearLinesAbove(streams, 1)
fmt.Fprintln(streams.Out, streams.ColorScheme().Green("✓ compatible remote builder created"))
Expand All @@ -329,6 +339,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam

remoteBuilderAppName := app.Name
remoteBuilderOrg := app.Organization.Slug
builderRegion := machine.Region

build.SetBuilderMetaPart1(remoteBuilderType, remoteBuilderAppName, machine.ID)

Expand Down Expand Up @@ -384,7 +395,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
if host == "" {
err = errors.New("machine did not have a private IP")
tracing.RecordError(span, err, "failed to boot remote builder")
return nil, err
return nil, "", err
}

builderHostOverride, ok := os.LookupEnv("FLY_RCHAB_OVERRIDE_HOST")
Expand All @@ -409,10 +420,10 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
captureError(err)

if strings.Contains(err.Error(), "timed out") || strings.Contains(err.Error(), "websocket") {
return nil, generateBrokenWGError(err)
return nil, "", generateBrokenWGError(err)
}

return nil, err
return nil, "", err
}

wireguardHttpClient, err := dockerclient.NewClientWithOpts(wireguardOpts...)
Expand All @@ -423,7 +434,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
captureError(err)
tracing.RecordError(span, err, "failed to initialize remote client")

return nil, err
return nil, "", err
}

cachedClient = wireguardHttpClient
Expand All @@ -434,7 +445,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam

err = fmt.Errorf("failed building wgless options: %w", err)
captureError(err)
return nil, err
return nil, "", err
}

wireguardlessHttpsClient, err := dockerclient.NewClientWithOpts(wglessOpts...)
Expand All @@ -445,7 +456,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
captureError(err)
tracing.RecordError(span, err, "failed to initialize wgLessHttpClient")

return nil, err
return nil, "", err
}
cachedClient = wireguardlessHttpsClient
}
Expand All @@ -459,18 +470,18 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
tracing.RecordError(span, err, "failed to wait for docker daemon")

if errors.Is(err, agent.ErrTunnelUnavailable) {
return nil, generateBrokenWGError(err)
return nil, "", generateBrokenWGError(err)
}

return nil, err
return nil, "", err
case !up:
streams.StopProgressIndicator()
err := errors.New("remote builder app unavailable")

terminal.Warnf("Remote builder did not start in time. Check remote builder logs with `flyctl logs -a %s`\n", remoteBuilderAppName)
tracing.RecordError(span, err, "remote builder failed to start")

return nil, err
return nil, "", err
default:
if msg := fmt.Sprintf("Remote builder %s ready", remoteBuilderAppName); streams.IsInteractive() {
streams.StopProgressIndicatorMsg(msg)
Expand All @@ -479,7 +490,7 @@ func newRemoteDockerClient(ctx context.Context, apiClient flyutil.Client, appNam
}
}

return cachedClient, nil
return cachedClient, builderRegion, nil
}

func generateBrokenWGError(err error) flyerr.GenericErr {
Expand Down Expand Up @@ -789,3 +800,32 @@ func (d *dockerClientFactory) IsRemote() bool {
func (d *dockerClientFactory) IsLocal() bool {
return !d.remote
}

// GetBuilderRegion returns the region of the builder
func (d *dockerClientFactory) GetBuilderRegion() string {
return d.builderRegion
}

// getBuilderInfo determines the builder type and region for metrics and logging
func getBuilderInfo(ctx context.Context, dockerFactory *dockerClientFactory) (builderType, builderRegion string) {
builderType = "local"
builderRegion = "unknown"

if dockerFactory.IsRemote() {
builderType = "remote"
builderRegion = dockerFactory.GetBuilderRegion()
if builderRegion == "" {
builderRegion = "remote"
}
} else {
// For local builders, try to get the nearest region
apiClient := flyutil.ClientFromContext(ctx)
if apiClient != nil {
if nearestRegion, err := apiClient.GetNearestRegion(ctx); err == nil {
builderRegion = nearestRegion.Code
}
}
}

return builderType, builderRegion
}
31 changes: 26 additions & 5 deletions internal/build/imgsrc/dockerfile_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ import (
"github.com/moby/buildkit/exporter/containerimage/exptypes"
"github.com/moby/buildkit/session/secrets/secretsprovider"
"github.com/pkg/errors"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"golang.org/x/sync/errgroup"

"github.com/superfly/flyctl/helpers"
"github.com/superfly/flyctl/internal/buildinfo"
"github.com/superfly/flyctl/internal/cmdfmt"
Expand All @@ -34,9 +38,6 @@ import (
"github.com/superfly/flyctl/internal/tracing"
"github.com/superfly/flyctl/iostreams"
"github.com/superfly/flyctl/terminal"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"golang.org/x/sync/errgroup"
)

type dockerfileBuilder struct{}
Expand Down Expand Up @@ -270,10 +271,12 @@ func (*dockerfileBuilder) Run(ctx context.Context, dockerFactory *dockerClientFa
build.BuildFinish()
cmdfmt.PrintDone(streams.ErrOut, "Building image done")

builderType, builderRegion := getBuilderInfo(ctx, dockerFactory)

if opts.Publish {
build.PushStart()
tb := render.NewTextBlock(ctx, "Pushing image to fly")
if err := pushToFly(ctx, docker, streams, opts.Tag); err != nil {
if err := pushToFly(ctx, docker, streams, opts.Tag, builderType, builderRegion); err != nil {
build.PushFinish()
return nil, "", err
}
Expand Down Expand Up @@ -521,7 +524,7 @@ func runBuildKitBuild(ctx context.Context, docker *dockerclient.Client, opts Ima
return res.ExporterResponse[exptypes.ExporterImageDigestKey], nil
}

func pushToFly(ctx context.Context, docker *dockerclient.Client, streams *iostreams.IOStreams, tag string) (err error) {
func pushToFly(ctx context.Context, docker *dockerclient.Client, streams *iostreams.IOStreams, tag string, builderType string, builderRegion string) (err error) {
ctx, span := tracing.GetTracer().Start(ctx, "push_image_to_registry", trace.WithAttributes(attribute.String("tag", tag)))
defer span.End()

Expand All @@ -531,6 +534,12 @@ func pushToFly(ctx context.Context, docker *dockerclient.Client, streams *iostre
}
}()

// Add builder type and region to span attributes
span.SetAttributes(
attribute.String("builder.type", builderType),
attribute.String("builder.region", builderRegion),
)

metrics.Started(ctx, "image_push")
sendImgPushMetrics := metrics.StartTiming(ctx, "image_push/duration")

Expand All @@ -543,6 +552,9 @@ func pushToFly(ctx context.Context, docker *dockerclient.Client, streams *iostre
return errors.Wrap(err, "error pushing image to registry")
}
defer pushResp.Close() // skipcq: GO-S2307

// Send enhanced metrics with builder type and region
SendEnhancedImagePushMetrics(ctx, builderType, builderRegion)
sendImgPushMetrics()

err = jsonmessage.DisplayJSONMessagesStream(pushResp, streams.ErrOut, streams.StderrFd(), streams.IsStderrTTY(), nil)
Expand All @@ -559,3 +571,12 @@ func pushToFly(ctx context.Context, docker *dockerclient.Client, streams *iostre

return nil
}

// SendEnhancedImagePushMetrics sends enhanced metrics for image push operations
// with builder type and region information
func SendEnhancedImagePushMetrics(ctx context.Context, builderType, builderRegion string) {
metrics.Send(ctx, "image_push/enhanced", map[string]interface{}{
"builder_type": builderType,
"builder_region": builderRegion,
})
}
Loading