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.

Sign up for GitHub

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

Jump to bottom

Add hidden gcptrace flag #1230

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+97 −30
Open

Add hidden gcptrace flag #1230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/md/melange_build.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ melange build [flags]
--source-dir string directory used for included sources
--strip-origin-name whether origin names should be stripped (for bootstrap)
--timeout duration default timeout for builds
--trace string where to write trace output
--vars-file string file to use for preloaded build configuration variables
--workspace-dir string directory used for the workspace at /home/build
```
Expand Down
3 changes: 3 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
chainguard.dev/apko v0.16.0
cloud.google.com/go/storage v1.43.0
dagger.io/dagger v0.11.9
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace v1.24.0
github.com/chainguard-dev/clog v1.4.0
github.com/chainguard-dev/go-pkgconfig v0.0.0-20240404163941-6351b37b2a10
github.com/chainguard-dev/yam v0.0.10
Expand Down Expand Up @@ -62,8 +63,10 @@ require (
cloud.google.com/go/auth/oauth2adapt v0.2.2 // indirect
cloud.google.com/go/compute/metadata v0.3.0 // indirect
cloud.google.com/go/iam v1.1.8 // indirect
cloud.google.com/go/trace v1.10.7 // indirect
github.com/99designs/gqlgen v0.17.46 // indirect
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.48.0 // indirect
github.com/Khan/genqlient v0.7.0 // indirect
github.com/MakeNowJust/heredoc/v2 v2.0.1 // indirect
github.com/Microsoft/go-winio v0.6.2 // indirect
Expand Down
12 changes: 12 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,16 @@ cloud.google.com/go/compute/metadata v0.3.0 h1:Tz+eQXMEqDIKRsmY3cHTL6FVaynIjX2Qx
cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k=
cloud.google.com/go/iam v1.1.8 h1:r7umDwhj+BQyz0ScZMp4QrGXjSTI3ZINnpgU2nlB/K0=
cloud.google.com/go/iam v1.1.8/go.mod h1:GvE6lyMmfxXauzNq8NbgJbeVQNspG+tcdL/W8QO1+zE=
cloud.google.com/go/logging v1.10.0 h1:f+ZXMqyrSJ5vZ5pE/zr0xC8y/M9BLNzQeLBwfeZ+wY4=
cloud.google.com/go/logging v1.10.0/go.mod h1:EHOwcxlltJrYGqMGfghSet736KR3hX1MAj614mrMk9I=
cloud.google.com/go/longrunning v0.5.7 h1:WLbHekDbjK1fVFD3ibpFFVoyizlLRl73I7YKuAKilhU=
cloud.google.com/go/longrunning v0.5.7/go.mod h1:8GClkudohy1Fxm3owmBGid8W0pSgodEMwEAztp38Xng=
cloud.google.com/go/monitoring v1.19.0 h1:NCXf8hfQi+Kmr56QJezXRZ6GPb80ZI7El1XztyUuLQI=
cloud.google.com/go/monitoring v1.19.0/go.mod h1:25IeMR5cQ5BoZ8j1eogHE5VPJLlReQ7zFp5OiLgiGZw=
cloud.google.com/go/storage v1.43.0 h1:CcxnSohZwizt4LCzQHWvBf1/kvtHUn7gk9QERXPyXFs=
cloud.google.com/go/storage v1.43.0/go.mod h1:ajvxEa7WmZS1PxvKRq4bq0tFT3vMd502JwstCcYv0Q0=
cloud.google.com/go/trace v1.10.7 h1:gK8z2BIJQ3KIYGddw9RJLne5Fx0FEXkrEQzPaeEYVvk=
cloud.google.com/go/trace v1.10.7/go.mod h1:qk3eiKmZX0ar2dzIJN/3QhY2PIFh1eqcIdaN5uEjQPM=
dagger.io/dagger v0.11.9 h1:CZ0spjR8psyy7khoRZA9gAKEgIfM2J2rWDtceRoBwNU=
dagger.io/dagger v0.11.9/go.mod h1:BJRGBPZbZs9MhbdI9s6Snwkn1xNT7b0xPEjBI57FC+8=
dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk=
Expand All @@ -24,6 +30,12 @@ github.com/99designs/gqlgen v0.17.46/go.mod h1:qRtiAeVPgkBBSPzZtoZXRRl5WkNrUTpp1
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25UVaW/CKtUDjefjrs0SPonmDGUVOYP0=
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace v1.24.0 h1:TBo1ql03qmVkZzEndpfkS4i9dOgCVvO0rQP7HEth110=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace v1.24.0/go.mod h1:pix4dhb6R3oDGZgQhkEGGC+5ZTz6kcxOhS4lhsSJhrE=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.48.0 h1:3vze4eFE3z2tDy2iSeI7yCQ17L8iLxN4OkXgvTr979s=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.48.0/go.mod h1:PdB0wkmILI+phhoBhWdrrB4LfORT9tHc03OOn+q3dWU=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.48.0 h1:ng6QH9Z4bAXCf0Z1cjR5hKESyc1BUiOrfIOhN+nHfRU=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.48.0/go.mod h1:ZC7rjqRzdhRKDK223jQ7Tsz89ZtrSSLH/VFzf7k5Sb0=
github.com/Khan/genqlient v0.7.0 h1:GZ1meyRnzcDTK48EjqB8t3bcfYvHArCUUvgOwpz1D4w=
github.com/Khan/genqlient v0.7.0/go.mod h1:HNyy3wZvuYwmW3Y7mkoQLZsa/R5n5yIRajS1kPBvSFM=
github.com/MakeNowJust/heredoc/v2 v2.0.1 h1:rlCHh70XXXv7toz95ajQWOWQnN4WNLt0TdpZYIR/J6A=
Expand Down
29 changes: 0 additions & 29 deletions pkg/cli/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ import (
"github.com/chainguard-dev/clog"
"github.com/spf13/cobra"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/exporters/stdout/stdouttrace"
"go.opentelemetry.io/otel/sdk/trace"
"golang.org/x/sync/errgroup"
)

Expand Down Expand Up @@ -76,8 +74,6 @@ func Build() *cobra.Command {
var libc string
var lintRequire, lintWarn []string

var traceFile string

cmd := &cobra.Command{
Use: "build",
Short: "Build a package from a YAML configuration file",
Expand All @@ -87,30 +83,6 @@ func Build() *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

if traceFile != "" {
w, err := os.Create(traceFile)
if err != nil {
return fmt.Errorf("creating trace file: %w", err)
}
defer w.Close()
exporter, err := stdouttrace.New(stdouttrace.WithWriter(w))
if err != nil {
return fmt.Errorf("creating stdout exporter: %w", err)
}
tp := trace.NewTracerProvider(trace.WithBatcher(exporter))
otel.SetTracerProvider(tp)

defer func() {
if err := tp.Shutdown(context.WithoutCancel(ctx)); err != nil {
clog.FromContext(ctx).Errorf("shutting down trace provider: %v", err)
}
}()

tctx, span := otel.Tracer("melange").Start(ctx, "build")
defer span.End()
ctx = tctx
}

r, err := getRunner(ctx, runner)
if err != nil {
return err
Expand Down Expand Up @@ -216,7 +188,6 @@ func Build() *cobra.Command {
cmd.Flags().StringVar(&cpu, "cpu", "", "default CPU resources to use for builds")
cmd.Flags().StringVar(&memory, "memory", "", "default memory resources to use for builds")
cmd.Flags().DurationVar(&timeout, "timeout", 0, "default timeout for builds")
cmd.Flags().StringVar(&traceFile, "trace", "", "where to write trace output")
cmd.Flags().StringSliceVar(&lintRequire, "lint-require", linter.DefaultRequiredLinters(), "linters that must pass")
cmd.Flags().StringSliceVar(&lintWarn, "lint-warn", linter.DefaultWarnLinters(), "linters that will generate warnings")

Expand Down
82 changes: 82 additions & 0 deletions pkg/cli/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,38 @@
package cli

import (
"context"
"errors"
"fmt"
"log/slog"
"net/http"
"os"
"slices"
"time"

"chainguard.dev/apko/pkg/log"
gtrace "github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace"
"github.com/chainguard-dev/clog/gcp"
charmlog "github.com/charmbracelet/log"
"github.com/spf13/cobra"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/exporters/stdout/stdouttrace"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
"sigs.k8s.io/release-utils/version"
)

func New() *cobra.Command {
var level log.CharmLogLevel
var gcplog bool
var gcptrace bool
var traceFile string

// Approximate calling defer() across Persistent[Pre/Post]RunE method calls.
stack := []func(context.Context) error{}
deferred := func(f func(context.Context) error) {
stack = append(stack, f)
}

cmd := &cobra.Command{
Use: "melange",
DisableAutoGenTag: true,
Expand All @@ -44,12 +61,77 @@ func New() *cobra.Command {
slog.SetDefault(slog.New(charmlog.NewWithOptions(os.Stderr, charmlog.Options{ReportTimestamp: true, Level: charmlog.Level(level)})))
}

var (
err error
exp sdktrace.SpanExporter
)

if gcptrace {
exp, err = gtrace.New()
if err != nil {
return fmt.Errorf("creating gcp trace exporter: %w", err)
}
} else if traceFile != "" {
w, err := os.Create(traceFile)
if err != nil {
return fmt.Errorf("creating trace file: %w", err)
}

deferred(func(context.Context) error {
return w.Close()
})

exp, err = stdouttrace.New(stdouttrace.WithWriter(w))
if err != nil {
return fmt.Errorf("creating stdout exporter: %w", err)
}
}

if exp != nil {
tp := sdktrace.NewTracerProvider(sdktrace.WithBatcher(exp))
otel.SetTracerProvider(tp)

deferred(func(ctx context.Context) error {
return tp.Shutdown(ctx)
})

deferred(func(ctx context.Context) error {
return tp.ForceFlush(ctx)
})

ctx, span := otel.Tracer("melange").Start(cmd.Context(), cmd.CalledAs())
cmd.SetContext(ctx)

deferred(func(context.Context) error {
span.End()
return nil
})
}

return nil
},
PersistentPostRunE: func(cmd *cobra.Command, args []string) error {
// It shouldn't take very long to flush spans but we don't want the whole build to fail if it takes a few seconds.
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

slices.Reverse(stack)

errs := []error{}
for _, f := range stack {
errs = append(errs, f(ctx))
}

return errors.Join(errs...)
},
}
cmd.PersistentFlags().Var(&level, "log-level", "log level (e.g. debug, info, warn, error)")
cmd.PersistentFlags().BoolVar(&gcplog, "gcplog", false, "use GCP logging")
_ = cmd.PersistentFlags().MarkHidden("gcplog")
cmd.PersistentFlags().BoolVar(&gcptrace, "gcptrace", false, "use GCP tracing")
_ = cmd.PersistentFlags().MarkHidden("gcptrace")
cmd.PersistentFlags().StringVar(&traceFile, "trace", "", "where to write trace output")
_ = cmd.PersistentFlags().MarkHidden("trace")
Comment on lines +133 to +134
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're marking this as hidden, could we have some documentation somewhere to show that it exists and how to use it? It's an incredible developer feature, IMHO

Copy link
Contributor Author

@jonjohnsonjr jonjohnsonjr Jul 11, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think basically nobody but me uses it, and I'd like to be able to break it in the future.

I'd also be fine un-hiding it if you think we should be more serious professionals.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to explicitly not commit to supporting this right now, which means (unfortunately) not telling anybody outside of this completely private circle of trust that it exists.

If it proves as useful as we think it is, and word catches on, we should un-hide it and document it.

I think a more ideal outcome would be that all the GCP trace exporting happens in a different, non-melange-CLI caller that wraps melange-as-a-library, since running the melange CLI in GCP directly as a long-term strategy is Bonkers™️.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with that more ideal outcome. And I agree that we shouldn't overcommit to supporting something externally.

I'm okay with leaving it hidden. I'd like to make a non-blocking request that we at least document this internally, if not now then soon. I'd love to move away from things that only one person gets to know about, even when the thing is not yet considered "stable".


cmd.AddCommand(Build())
cmd.AddCommand(Bump())
Expand Down