From d6b5ae37108b0f156b43363e948d66fd02a87777 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 20 May 2019 14:33:37 -0700 Subject: [PATCH] Remove usage of ocstatus package This changes the tracing package to accept an error on SetStatus, which is really what we always want anyway. This also decouples the trace package from opencensus. --- Gopkg.toml | 4 ---- providers/azure/metrics.go | 7 +++---- providers/mock/mock.go | 3 +-- trace/nop.go | 7 +++---- trace/opencensus/opencensus.go | 26 +++++++++++++++++++++++++- trace/trace.go | 15 +++++++-------- vkubelet/node.go | 7 +++---- vkubelet/pod.go | 19 +++++++++---------- vkubelet/podcontroller.go | 15 +++++++-------- vkubelet/queue.go | 3 +-- 10 files changed, 59 insertions(+), 47 deletions(-) diff --git a/Gopkg.toml b/Gopkg.toml index 15856e36a..952f82398 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -70,10 +70,6 @@ name = "github.com/gophercloud/gophercloud" revision = "954aa14363ced787c28efcfcd15ae6945eb862fb" -[[constraint]] - name = "github.com/cpuguy83/strongerrors" - version = "0.2.1" - [[override]] name = "github.com/docker/docker" revision = "49bf474f9ed7ce7143a59d1964ff7b7fd9b52178" diff --git a/providers/azure/metrics.go b/providers/azure/metrics.go index eeb140d4a..893fe1c43 100644 --- a/providers/azure/metrics.go +++ b/providers/azure/metrics.go @@ -5,7 +5,6 @@ import ( "strings" "time" - "github.com/cpuguy83/strongerrors/status/ocstatus" "github.com/pkg/errors" "github.com/virtual-kubelet/azure-aci/client/aci" "github.com/virtual-kubelet/virtual-kubelet/log" @@ -97,7 +96,7 @@ func (p *ACIProvider) GetStatsSummary(ctx context.Context) (summary *stats.Summa Types: []aci.MetricType{aci.MetricTypeCPUUsage, aci.MetricTypeMemoryUsage}, }) if err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return errors.Wrapf(err, "error fetching cpu/mem stats for container group %s", cgName) } logger.Debug("Got system stats") @@ -109,7 +108,7 @@ func (p *ACIProvider) GetStatsSummary(ctx context.Context) (summary *stats.Summa Types: []aci.MetricType{aci.MetricTyperNetworkBytesRecievedPerSecond, aci.MetricTyperNetworkBytesTransmittedPerSecond}, }) if err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return errors.Wrapf(err, "error fetching network stats for container group %s", cgName) } logger.Debug("Got network stats") @@ -120,7 +119,7 @@ func (p *ACIProvider) GetStatsSummary(ctx context.Context) (summary *stats.Summa } if err := errGroup.Wait(); err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return nil, errors.Wrap(err, "error in request to fetch container group metrics") } close(chResult) diff --git a/providers/mock/mock.go b/providers/mock/mock.go index 436a6abc6..6e40e60e6 100644 --- a/providers/mock/mock.go +++ b/providers/mock/mock.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/cpuguy83/strongerrors/status/ocstatus" "github.com/virtual-kubelet/virtual-kubelet/errdefs" "github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/trace" @@ -201,7 +200,7 @@ func (p *MockProvider) DeletePod(ctx context.Context, pod *v1.Pod) (err error) { func (p *MockProvider) GetPod(ctx context.Context, namespace, name string) (pod *v1.Pod, err error) { ctx, span := trace.StartSpan(ctx, "GetPod") defer func() { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) span.End() }() diff --git a/trace/nop.go b/trace/nop.go index 2954ec63c..7a50bcf61 100644 --- a/trace/nop.go +++ b/trace/nop.go @@ -4,7 +4,6 @@ import ( "context" "github.com/virtual-kubelet/virtual-kubelet/log" - "go.opencensus.io/trace" ) type nopTracer struct{} @@ -15,9 +14,9 @@ func (nopTracer) StartSpan(ctx context.Context, _ string) (context.Context, Span type nopSpan struct{} -func (nopSpan) End() {} -func (nopSpan) SetStatus(trace.Status) {} -func (nopSpan) Logger() log.Logger { return nil } +func (nopSpan) End() {} +func (nopSpan) SetStatus(error) {} +func (nopSpan) Logger() log.Logger { return nil } func (nopSpan) WithField(ctx context.Context, _ string, _ interface{}) context.Context { return ctx } func (nopSpan) WithFields(ctx context.Context, _ log.Fields) context.Context { return ctx } diff --git a/trace/opencensus/opencensus.go b/trace/opencensus/opencensus.go index afaef4f4d..ae9023d4b 100644 --- a/trace/opencensus/opencensus.go +++ b/trace/opencensus/opencensus.go @@ -9,6 +9,7 @@ import ( "fmt" "sync" + "github.com/virtual-kubelet/virtual-kubelet/errdefs" "github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/trace" octrace "go.opencensus.io/trace" @@ -46,7 +47,30 @@ func (s *span) End() { s.s.End() } -func (s *span) SetStatus(status trace.Status) { +func (s *span) SetStatus(err error) { + if !s.s.IsRecordingEvents() { + return + } + + var status octrace.Status + + if err == nil { + status.Code = octrace.StatusCodeOK + s.s.SetStatus(status) + return + } + + switch { + case errdefs.IsNotFound(err): + status.Code = octrace.StatusCodeNotFound + case errdefs.IsInvalidInput(err): + status.Code = octrace.StatusCodeInvalidArgument + // TODO: other error types + default: + status.Code = octrace.StatusCodeUnknown + } + + status.Message = err.Error() s.s.SetStatus(status) } diff --git a/trace/trace.go b/trace/trace.go index 752432d4c..27e3ce27e 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -9,15 +9,8 @@ import ( "context" "github.com/virtual-kubelet/virtual-kubelet/log" - "go.opencensus.io/trace" ) -// Status is an alias to opencensus's trace status. -// The main reason we use this instead of implementing our own is library re-use, -// namely for converting an error to a tracing status. -// In the future this may be defined completely in this package. -type Status = trace.Status - // Tracer is the interface used for creating a tracing span type Tracer interface { // StartSpan starts a new span. The span details are emebedded into the returned @@ -41,7 +34,13 @@ func StartSpan(ctx context.Context, name string) (context.Context, Span) { // Span encapsulates a tracing event type Span interface { End() - SetStatus(Status) + + // SetStatus sets the final status of the span. + // errors passed to this should use interfaces defined in + // github.com/virtual-kubelet/virtual-kubelet/errdefs + // + // If the error is nil, the span should be considered successful. + SetStatus(err error) // WithField and WithFields adds attributes to an entire span // diff --git a/vkubelet/node.go b/vkubelet/node.go index 12c1923cf..4fa73bc21 100644 --- a/vkubelet/node.go +++ b/vkubelet/node.go @@ -6,7 +6,6 @@ import ( "fmt" "time" - "github.com/cpuguy83/strongerrors/status/ocstatus" pkgerrors "github.com/pkg/errors" "github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/trace" @@ -275,7 +274,7 @@ func (n *Node) handlePing(ctx context.Context) (retErr error) { ctx, span := trace.StartSpan(ctx, "node.handlePing") defer span.End() defer func() { - span.SetStatus(ocstatus.FromError(retErr)) + span.SetStatus(retErr) }() if err := n.p.Ping(ctx); err != nil { @@ -367,7 +366,7 @@ func UpdateNodeLease(ctx context.Context, leases v1beta1.LeaseInterface, lease * l, err = ensureLease(ctx, leases, lease) } if err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return nil, err } log.G(ctx).Debug("created new lease") @@ -429,7 +428,7 @@ func UpdateNodeStatus(ctx context.Context, nodes v1.NodeInterface, n *corev1.Nod ctx, span := trace.StartSpan(ctx, "UpdateNodeStatus") defer func() { span.End() - span.SetStatus(ocstatus.FromError(retErr)) + span.SetStatus(retErr) }() var node *corev1.Node diff --git a/vkubelet/pod.go b/vkubelet/pod.go index b0a31c010..2473ebbfc 100644 --- a/vkubelet/pod.go +++ b/vkubelet/pod.go @@ -5,7 +5,6 @@ import ( "hash/fnv" "time" - "github.com/cpuguy83/strongerrors/status/ocstatus" "github.com/davecgh/go-spew/spew" pkgerrors "github.com/pkg/errors" "github.com/virtual-kubelet/virtual-kubelet/log" @@ -44,7 +43,7 @@ func (pc *PodController) createOrUpdatePod(ctx context.Context, pod *corev1.Pod) }) if err := populateEnvironmentVariables(ctx, pod, pc.resourceManager, pc.recorder); err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return err } @@ -113,7 +112,7 @@ func (pc *PodController) handleProviderError(ctx context.Context, span trace.Spa } else { logger.Info("Updated k8s pod status") } - span.SetStatus(ocstatus.FromError(origErr)) + span.SetStatus(origErr) } func (pc *PodController) deletePod(ctx context.Context, namespace, name string) error { @@ -132,7 +131,7 @@ func (pc *PodController) deletePod(ctx context.Context, namespace, name string) var delErr error if delErr = pc.provider.DeletePod(ctx, pod); delErr != nil && errors.IsNotFound(delErr) { - span.SetStatus(ocstatus.FromError(delErr)) + span.SetStatus(delErr) return delErr } @@ -140,7 +139,7 @@ func (pc *PodController) deletePod(ctx context.Context, namespace, name string) if !errors.IsNotFound(delErr) { if err := pc.forceDeletePodResource(ctx, namespace, name); err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return err } log.G(ctx).Info("Deleted pod from Kubernetes") @@ -163,7 +162,7 @@ func (pc *PodController) forceDeletePodResource(ctx context.Context, namespace, log.G(ctx).Debug("Pod does not exist in Kubernetes, nothing to delete") return nil } - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return pkgerrors.Wrap(err, "Failed to delete Kubernetes pod") } return nil @@ -178,7 +177,7 @@ func (pc *PodController) updatePodStatuses(ctx context.Context, q workqueue.Rate pods, err := pc.podsLister.List(labels.Everything()) if err != nil { err = pkgerrors.Wrap(err, "error getting pod list") - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) log.G(ctx).WithError(err).Error("Error updating pod statuses") return } @@ -208,7 +207,7 @@ func (pc *PodController) updatePodStatus(ctx context.Context, pod *corev1.Pod) e status, err := pc.provider.GetPodStatus(ctx, pod.Namespace, pod.Name) if err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return pkgerrors.Wrap(err, "error retreiving pod status") } @@ -238,7 +237,7 @@ func (pc *PodController) updatePodStatus(ctx context.Context, pod *corev1.Pod) e } if _, err := pc.client.Pods(pod.Namespace).UpdateStatus(pod); err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return pkgerrors.Wrap(err, "error while updating pod status in kubernetes") } @@ -265,7 +264,7 @@ func (pc *PodController) podStatusHandler(ctx context.Context, key string) (retE ctx = span.WithField(ctx, "key", key) log.G(ctx).Debug("processing pod status update") defer func() { - span.SetStatus(ocstatus.FromError(retErr)) + span.SetStatus(retErr) if retErr != nil { log.G(ctx).WithError(retErr).Error("Error processing pod status update") } diff --git a/vkubelet/podcontroller.go b/vkubelet/podcontroller.go index 96205216b..503c78f8b 100644 --- a/vkubelet/podcontroller.go +++ b/vkubelet/podcontroller.go @@ -22,7 +22,6 @@ import ( "sync" "time" - "github.com/cpuguy83/strongerrors/status/ocstatus" pkgerrors "github.com/pkg/errors" "github.com/virtual-kubelet/virtual-kubelet/errdefs" "github.com/virtual-kubelet/virtual-kubelet/log" @@ -261,14 +260,14 @@ func (pc *PodController) syncHandler(ctx context.Context, key string) error { // We've failed to fetch the pod from the lister, but the error is not a 404. // Hence, we add the key back to the work queue so we can retry processing it later. err := pkgerrors.Wrapf(err, "failed to fetch pod with key %q from lister", key) - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return err } // At this point we know the Pod resource doesn't exist, which most probably means it was deleted. // Hence, we must delete it from the provider if it still exists there. if err := pc.deletePod(ctx, namespace, name); err != nil { err := pkgerrors.Wrapf(err, "failed to delete pod %q in the provider", loggablePodNameFromCoordinates(namespace, name)) - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return err } return nil @@ -290,7 +289,7 @@ func (pc *PodController) syncPodInProvider(ctx context.Context, pod *corev1.Pod) if pod.DeletionTimestamp != nil { if err := pc.deletePod(ctx, pod.Namespace, pod.Name); err != nil { err := pkgerrors.Wrapf(err, "failed to delete pod %q in the provider", loggablePodName(pod)) - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return err } return nil @@ -305,7 +304,7 @@ func (pc *PodController) syncPodInProvider(ctx context.Context, pod *corev1.Pod) // Create or update the pod in the provider. if err := pc.createOrUpdatePod(ctx, pod); err != nil { err := pkgerrors.Wrapf(err, "failed to sync pod %q in the provider", loggablePodName(pod)) - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) return err } return nil @@ -320,7 +319,7 @@ func (pc *PodController) deleteDanglingPods(ctx context.Context, threadiness int pps, err := pc.provider.GetPods(ctx) if err != nil { err := pkgerrors.Wrap(err, "failed to fetch the list of pods from the provider") - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) log.G(ctx).Error(err) return } @@ -339,7 +338,7 @@ func (pc *PodController) deleteDanglingPods(ctx context.Context, threadiness int } // For some reason we couldn't fetch the pod from the lister, so we propagate the error. err := pkgerrors.Wrap(err, "failed to fetch pod from the lister") - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) log.G(ctx).Error(err) return } @@ -367,7 +366,7 @@ func (pc *PodController) deleteDanglingPods(ctx context.Context, threadiness int ctx = addPodAttributes(ctx, span, pod) // Actually delete the pod. if err := pc.deletePod(ctx, pod.Namespace, pod.Name); err != nil { - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) log.G(ctx).Errorf("failed to delete pod %q in provider", loggablePodName(pod)) } else { log.G(ctx).Infof("deleted leaked pod %q in provider", loggablePodName(pod)) diff --git a/vkubelet/queue.go b/vkubelet/queue.go index 790f07a50..854fe4d39 100644 --- a/vkubelet/queue.go +++ b/vkubelet/queue.go @@ -5,7 +5,6 @@ import ( "strconv" "time" - "github.com/cpuguy83/strongerrors/status/ocstatus" pkgerrors "github.com/pkg/errors" "github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/trace" @@ -71,7 +70,7 @@ func handleQueueItem(ctx context.Context, q workqueue.RateLimitingInterface, han if err != nil { // We've actually hit an error, so we set the span's status based on the error. - span.SetStatus(ocstatus.FromError(err)) + span.SetStatus(err) log.G(ctx).Error(err) return true }