diff --git a/pkg/agent/binary.go b/pkg/agent/binary.go index bb003c90b..b705d2c77 100644 --- a/pkg/agent/binary.go +++ b/pkg/agent/binary.go @@ -39,16 +39,16 @@ func NewBinaryManager(logger log.Logger, downloadURL string) *BinaryManager { } } -func (m *BinaryManager) AcquireBinary(ctx context.Context, arch string) (io.ReadCloser, string, error) { +func (m *BinaryManager) AcquireBinary(ctx context.Context, arch string) (io.ReadCloser, error) { for _, source := range m.sources { binary, err := source.GetBinary(ctx, arch) if err == nil { m.logger.Debugf("acquired binary from %s", source.SourceName()) - return binary, source.SourceName(), nil + return binary, nil } m.logger.Debugf("source %s failed: %v", source.SourceName(), err) } - return nil, "", ErrBinaryNotFound + return nil, ErrBinaryNotFound } type BinaryCache struct { diff --git a/pkg/agent/inject.go b/pkg/agent/inject.go index 5f4d7ae96..0667de2f0 100644 --- a/pkg/agent/inject.go +++ b/pkg/agent/inject.go @@ -71,8 +71,6 @@ type InjectOptions struct { // SkipVersionCheck disables the validation of the remote agent's version. // Defaults to false, unless DEVPOD_AGENT_URL is set. SkipVersionCheck bool - // MetricsCollector handles the recording of injection metrics. Defaults to LogMetricsCollector. - MetricsCollector MetricsCollector } func (o *InjectOptions) ApplyDefaults() { @@ -164,27 +162,10 @@ func InjectAgent(opts *InjectOptions) error { return err } - if opts.MetricsCollector == nil { - opts.MetricsCollector = &LogMetricsCollector{Log: opts.Log} - } - metrics := &InjectionMetrics{StartTime: time.Now(), BinarySource: "existing"} - defer func() { - metrics.EndTime = time.Now() - opts.MetricsCollector.RecordInjection(metrics) - }() - if opts.IsLocal { return injectLocally(opts) } - opts.Log.WithFields(logrus.Fields{ - "localVersion": opts.LocalVersion, - "remoteVersion": opts.RemoteVersion, - "skipCheck": opts.SkipVersionCheck, - "preferDownload": strconv.FormatBool(*opts.PreferDownloadFromRemoteUrl), - "timeout": opts.Timeout, - }).Debug("starting agent injection") - vc := newVersionChecker(opts) bm := NewBinaryManager(opts.Log, opts.DownloadURL) @@ -196,21 +177,18 @@ func InjectAgent(opts *InjectOptions) error { Cap: 60 * time.Second, } - attempt := 0 + opts.Log.Debug("starting agent injection") return retry.OnError(backoff, func(err error) bool { if opts.Ctx.Err() != nil { return false } - opts.Log.Debugf("retrying attempt %d: %v", attempt, err) + opts.Log.Debugf("retrying injection: %v", err) return true }, func() error { - attempt++ return injectAgent(&injectContext{ - attempt: attempt, - opts: opts, - bm: bm, - vc: vc, - metrics: metrics, + opts: opts, + bm: bm, + vc: vc, }) }) } @@ -224,17 +202,13 @@ func injectLocally(opts *InjectOptions) error { } type injectContext struct { - attempt int - opts *InjectOptions - bm *BinaryManager - vc *versionChecker - metrics *InjectionMetrics + opts *InjectOptions + bm *BinaryManager + vc *versionChecker } func injectAgent(ctx *injectContext) error { opts := ctx.opts - metrics := ctx.metrics - metrics.Attempts = ctx.attempt buf := &bytes.Buffer{} stderr := setupStderr(opts, buf) @@ -254,7 +228,7 @@ func injectAgent(ctx *injectContext) error { }) if err != nil { - return handleInjectError(err, wasExecuted, buf, metrics) + return handleInjectError(err, wasExecuted, buf) } return performVersionCheck(ctx) @@ -273,12 +247,7 @@ func createBinaryLoader(ctx *injectContext) func(bool) (io.ReadCloser, error) { if arm { arch = "arm64" } - stream, source, err := ctx.bm.AcquireBinary(ctx.opts.Ctx, arch) - if err != nil { - return nil, err - } - ctx.metrics.BinarySource = source - return stream, nil + return ctx.bm.AcquireBinary(ctx.opts.Ctx, arch) } } @@ -294,40 +263,44 @@ func buildScriptParams(ctx *injectContext) *inject.Params { } } -func handleInjectError(err error, wasExecuted bool, buf *bytes.Buffer, metrics *InjectionMetrics) error { - metrics.Error = err +func handleInjectError(err error, wasExecuted bool, buf *bytes.Buffer) error { if wasExecuted { return &InjectError{ - Stage: "command_exec", + Stage: InjectStageCommandExecution, Cause: fmt.Errorf("%w: %s", err, buf.String()), } } - return &InjectError{Stage: "inject", Cause: err} + return &InjectError{Stage: InjectStageInject, Cause: err} } func performVersionCheck(ctx *injectContext) error { opts := ctx.opts - metrics := ctx.metrics detectedVersion, err := ctx.vc.detectRemoteAgentVersion(opts.Ctx, opts.Exec, opts.RemoteAgentPath, opts.Log) - if detectedVersion != "" { - metrics.AgentVersion = detectedVersion - } if !opts.SkipVersionCheck { if err != nil { - metrics.VersionCheck = false - return &InjectError{Stage: "version_check", Cause: err} + return &InjectError{Stage: InjectStageVersionCheck, Cause: err} } - metrics.VersionCheck = true } - metrics.Success = true + if detectedVersion != "" && !opts.SkipVersionCheck { + opts.Log.Debugf("detected remote agent version: %s", detectedVersion) + } + return nil } +type InjectStage string + +const ( + InjectStageInject InjectStage = "inject" + InjectStageCommandExecution InjectStage = "command execution" + InjectStageVersionCheck InjectStage = "version check" +) + type InjectError struct { - Stage string + Stage InjectStage Cause error } @@ -342,36 +315,6 @@ func (e *InjectError) Unwrap() error { return e.Cause } -type InjectionMetrics struct { - StartTime time.Time - EndTime time.Time - Attempts int - BinarySource string - AgentVersion string - VersionCheck bool - Success bool - Error error -} - -type MetricsCollector interface { - RecordInjection(metrics *InjectionMetrics) -} - -type LogMetricsCollector struct { - Log log.Logger -} - -func (c *LogMetricsCollector) RecordInjection(metrics *InjectionMetrics) { - c.Log.WithFields(logrus.Fields{ - "duration": metrics.EndTime.Sub(metrics.StartTime), - "attempts": metrics.Attempts, - "binarySource": metrics.BinarySource, - "remoteAgentVersion": metrics.AgentVersion, - "versionCheck": metrics.VersionCheck, - "success": metrics.Success, - }).Debug("agent injection metrics") -} - type versionChecker struct { localVersion string remoteVersion string @@ -423,11 +366,7 @@ func (vc *versionChecker) detectRemoteAgentVersion( "If your workspace fails to deploy, you may need to manually remove " + "the existing agent and redeploy.") } else { - log.WithFields(logrus.Fields{ - "expected": vc.remoteVersion, - "actual": actualVersion, - "agentPath": agentPath, - }).Debug("remote agent version validated") + log.Debug("remote agent version matches expected version") } return actualVersion, nil