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.
This commit is contained in:
Brian Goff
2019-05-20 14:33:37 -07:00
parent 02623170cc
commit d6b5ae3710
10 changed files with 59 additions and 47 deletions

View File

@@ -70,10 +70,6 @@
name = "github.com/gophercloud/gophercloud" name = "github.com/gophercloud/gophercloud"
revision = "954aa14363ced787c28efcfcd15ae6945eb862fb" revision = "954aa14363ced787c28efcfcd15ae6945eb862fb"
[[constraint]]
name = "github.com/cpuguy83/strongerrors"
version = "0.2.1"
[[override]] [[override]]
name = "github.com/docker/docker" name = "github.com/docker/docker"
revision = "49bf474f9ed7ce7143a59d1964ff7b7fd9b52178" revision = "49bf474f9ed7ce7143a59d1964ff7b7fd9b52178"

View File

@@ -5,7 +5,6 @@ import (
"strings" "strings"
"time" "time"
"github.com/cpuguy83/strongerrors/status/ocstatus"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/virtual-kubelet/azure-aci/client/aci" "github.com/virtual-kubelet/azure-aci/client/aci"
"github.com/virtual-kubelet/virtual-kubelet/log" "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}, Types: []aci.MetricType{aci.MetricTypeCPUUsage, aci.MetricTypeMemoryUsage},
}) })
if err != nil { 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) return errors.Wrapf(err, "error fetching cpu/mem stats for container group %s", cgName)
} }
logger.Debug("Got system stats") 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}, Types: []aci.MetricType{aci.MetricTyperNetworkBytesRecievedPerSecond, aci.MetricTyperNetworkBytesTransmittedPerSecond},
}) })
if err != nil { if err != nil {
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
return errors.Wrapf(err, "error fetching network stats for container group %s", cgName) return errors.Wrapf(err, "error fetching network stats for container group %s", cgName)
} }
logger.Debug("Got network stats") 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 { 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") return nil, errors.Wrap(err, "error in request to fetch container group metrics")
} }
close(chResult) close(chResult)

View File

@@ -10,7 +10,6 @@ import (
"strings" "strings"
"time" "time"
"github.com/cpuguy83/strongerrors/status/ocstatus"
"github.com/virtual-kubelet/virtual-kubelet/errdefs" "github.com/virtual-kubelet/virtual-kubelet/errdefs"
"github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/log"
"github.com/virtual-kubelet/virtual-kubelet/trace" "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) { func (p *MockProvider) GetPod(ctx context.Context, namespace, name string) (pod *v1.Pod, err error) {
ctx, span := trace.StartSpan(ctx, "GetPod") ctx, span := trace.StartSpan(ctx, "GetPod")
defer func() { defer func() {
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
span.End() span.End()
}() }()

View File

@@ -4,7 +4,6 @@ import (
"context" "context"
"github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/log"
"go.opencensus.io/trace"
) )
type nopTracer struct{} type nopTracer struct{}
@@ -15,9 +14,9 @@ func (nopTracer) StartSpan(ctx context.Context, _ string) (context.Context, Span
type nopSpan struct{} type nopSpan struct{}
func (nopSpan) End() {} func (nopSpan) End() {}
func (nopSpan) SetStatus(trace.Status) {} func (nopSpan) SetStatus(error) {}
func (nopSpan) Logger() log.Logger { return nil } func (nopSpan) Logger() log.Logger { return nil }
func (nopSpan) WithField(ctx context.Context, _ string, _ interface{}) context.Context { return ctx } func (nopSpan) WithField(ctx context.Context, _ string, _ interface{}) context.Context { return ctx }
func (nopSpan) WithFields(ctx context.Context, _ log.Fields) context.Context { return ctx } func (nopSpan) WithFields(ctx context.Context, _ log.Fields) context.Context { return ctx }

View File

@@ -9,6 +9,7 @@ import (
"fmt" "fmt"
"sync" "sync"
"github.com/virtual-kubelet/virtual-kubelet/errdefs"
"github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/log"
"github.com/virtual-kubelet/virtual-kubelet/trace" "github.com/virtual-kubelet/virtual-kubelet/trace"
octrace "go.opencensus.io/trace" octrace "go.opencensus.io/trace"
@@ -46,7 +47,30 @@ func (s *span) End() {
s.s.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) s.s.SetStatus(status)
} }

View File

@@ -9,15 +9,8 @@ import (
"context" "context"
"github.com/virtual-kubelet/virtual-kubelet/log" "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 // Tracer is the interface used for creating a tracing span
type Tracer interface { type Tracer interface {
// StartSpan starts a new span. The span details are emebedded into the returned // 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 // Span encapsulates a tracing event
type Span interface { type Span interface {
End() 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 // WithField and WithFields adds attributes to an entire span
// //

View File

@@ -6,7 +6,6 @@ import (
"fmt" "fmt"
"time" "time"
"github.com/cpuguy83/strongerrors/status/ocstatus"
pkgerrors "github.com/pkg/errors" pkgerrors "github.com/pkg/errors"
"github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/log"
"github.com/virtual-kubelet/virtual-kubelet/trace" "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") ctx, span := trace.StartSpan(ctx, "node.handlePing")
defer span.End() defer span.End()
defer func() { defer func() {
span.SetStatus(ocstatus.FromError(retErr)) span.SetStatus(retErr)
}() }()
if err := n.p.Ping(ctx); err != nil { 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) l, err = ensureLease(ctx, leases, lease)
} }
if err != nil { if err != nil {
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
return nil, err return nil, err
} }
log.G(ctx).Debug("created new lease") 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") ctx, span := trace.StartSpan(ctx, "UpdateNodeStatus")
defer func() { defer func() {
span.End() span.End()
span.SetStatus(ocstatus.FromError(retErr)) span.SetStatus(retErr)
}() }()
var node *corev1.Node var node *corev1.Node

View File

@@ -5,7 +5,6 @@ import (
"hash/fnv" "hash/fnv"
"time" "time"
"github.com/cpuguy83/strongerrors/status/ocstatus"
"github.com/davecgh/go-spew/spew" "github.com/davecgh/go-spew/spew"
pkgerrors "github.com/pkg/errors" pkgerrors "github.com/pkg/errors"
"github.com/virtual-kubelet/virtual-kubelet/log" "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 { if err := populateEnvironmentVariables(ctx, pod, pc.resourceManager, pc.recorder); err != nil {
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
return err return err
} }
@@ -113,7 +112,7 @@ func (pc *PodController) handleProviderError(ctx context.Context, span trace.Spa
} else { } else {
logger.Info("Updated k8s pod status") 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 { 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 var delErr error
if delErr = pc.provider.DeletePod(ctx, pod); delErr != nil && errors.IsNotFound(delErr) { if delErr = pc.provider.DeletePod(ctx, pod); delErr != nil && errors.IsNotFound(delErr) {
span.SetStatus(ocstatus.FromError(delErr)) span.SetStatus(delErr)
return delErr return delErr
} }
@@ -140,7 +139,7 @@ func (pc *PodController) deletePod(ctx context.Context, namespace, name string)
if !errors.IsNotFound(delErr) { if !errors.IsNotFound(delErr) {
if err := pc.forceDeletePodResource(ctx, namespace, name); err != nil { if err := pc.forceDeletePodResource(ctx, namespace, name); err != nil {
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
return err return err
} }
log.G(ctx).Info("Deleted pod from Kubernetes") 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") log.G(ctx).Debug("Pod does not exist in Kubernetes, nothing to delete")
return nil return nil
} }
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
return pkgerrors.Wrap(err, "Failed to delete Kubernetes pod") return pkgerrors.Wrap(err, "Failed to delete Kubernetes pod")
} }
return nil return nil
@@ -178,7 +177,7 @@ func (pc *PodController) updatePodStatuses(ctx context.Context, q workqueue.Rate
pods, err := pc.podsLister.List(labels.Everything()) pods, err := pc.podsLister.List(labels.Everything())
if err != nil { if err != nil {
err = pkgerrors.Wrap(err, "error getting pod list") 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") log.G(ctx).WithError(err).Error("Error updating pod statuses")
return 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) status, err := pc.provider.GetPodStatus(ctx, pod.Namespace, pod.Name)
if err != nil { if err != nil {
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
return pkgerrors.Wrap(err, "error retreiving pod status") 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 { 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") 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) ctx = span.WithField(ctx, "key", key)
log.G(ctx).Debug("processing pod status update") log.G(ctx).Debug("processing pod status update")
defer func() { defer func() {
span.SetStatus(ocstatus.FromError(retErr)) span.SetStatus(retErr)
if retErr != nil { if retErr != nil {
log.G(ctx).WithError(retErr).Error("Error processing pod status update") log.G(ctx).WithError(retErr).Error("Error processing pod status update")
} }

View File

@@ -22,7 +22,6 @@ import (
"sync" "sync"
"time" "time"
"github.com/cpuguy83/strongerrors/status/ocstatus"
pkgerrors "github.com/pkg/errors" pkgerrors "github.com/pkg/errors"
"github.com/virtual-kubelet/virtual-kubelet/errdefs" "github.com/virtual-kubelet/virtual-kubelet/errdefs"
"github.com/virtual-kubelet/virtual-kubelet/log" "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. // 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. // 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) err := pkgerrors.Wrapf(err, "failed to fetch pod with key %q from lister", key)
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
return err return err
} }
// At this point we know the Pod resource doesn't exist, which most probably means it was deleted. // 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. // Hence, we must delete it from the provider if it still exists there.
if err := pc.deletePod(ctx, namespace, name); err != nil { if err := pc.deletePod(ctx, namespace, name); err != nil {
err := pkgerrors.Wrapf(err, "failed to delete pod %q in the provider", loggablePodNameFromCoordinates(namespace, name)) 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 err
} }
return nil return nil
@@ -290,7 +289,7 @@ func (pc *PodController) syncPodInProvider(ctx context.Context, pod *corev1.Pod)
if pod.DeletionTimestamp != nil { if pod.DeletionTimestamp != nil {
if err := pc.deletePod(ctx, pod.Namespace, pod.Name); err != 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)) 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 err
} }
return nil return nil
@@ -305,7 +304,7 @@ func (pc *PodController) syncPodInProvider(ctx context.Context, pod *corev1.Pod)
// Create or update the pod in the provider. // Create or update the pod in the provider.
if err := pc.createOrUpdatePod(ctx, pod); err != nil { if err := pc.createOrUpdatePod(ctx, pod); err != nil {
err := pkgerrors.Wrapf(err, "failed to sync pod %q in the provider", loggablePodName(pod)) 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 err
} }
return nil return nil
@@ -320,7 +319,7 @@ func (pc *PodController) deleteDanglingPods(ctx context.Context, threadiness int
pps, err := pc.provider.GetPods(ctx) pps, err := pc.provider.GetPods(ctx)
if err != nil { if err != nil {
err := pkgerrors.Wrap(err, "failed to fetch the list of pods from the provider") 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) log.G(ctx).Error(err)
return 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. // 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") err := pkgerrors.Wrap(err, "failed to fetch pod from the lister")
span.SetStatus(ocstatus.FromError(err)) span.SetStatus(err)
log.G(ctx).Error(err) log.G(ctx).Error(err)
return return
} }
@@ -367,7 +366,7 @@ func (pc *PodController) deleteDanglingPods(ctx context.Context, threadiness int
ctx = addPodAttributes(ctx, span, pod) ctx = addPodAttributes(ctx, span, pod)
// Actually delete the pod. // Actually delete the pod.
if err := pc.deletePod(ctx, pod.Namespace, pod.Name); err != nil { 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)) log.G(ctx).Errorf("failed to delete pod %q in provider", loggablePodName(pod))
} else { } else {
log.G(ctx).Infof("deleted leaked pod %q in provider", loggablePodName(pod)) log.G(ctx).Infof("deleted leaked pod %q in provider", loggablePodName(pod))

View File

@@ -5,7 +5,6 @@ import (
"strconv" "strconv"
"time" "time"
"github.com/cpuguy83/strongerrors/status/ocstatus"
pkgerrors "github.com/pkg/errors" pkgerrors "github.com/pkg/errors"
"github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/log"
"github.com/virtual-kubelet/virtual-kubelet/trace" "github.com/virtual-kubelet/virtual-kubelet/trace"
@@ -71,7 +70,7 @@ func handleQueueItem(ctx context.Context, q workqueue.RateLimitingInterface, han
if err != nil { if err != nil {
// We've actually hit an error, so we set the span's status based on the error. // 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) log.G(ctx).Error(err)
return true return true
} }