Removing spans from guest and other areas

Removing unnecessary spans in the codebase and replacing with trace logs

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>

internal/guest/bridge/bridge.go

@@ -17,15 +17,16 @@ import (
  "sync/atomic"
  "time"
 
+ "github.com/pkg/errors"
+ "github.com/sirupsen/logrus"
+ "go.opencensus.io/trace"
+ "go.opencensus.io/trace/tracestate"
+
  "github.com/Microsoft/hcsshim/internal/guest/gcserr"
  "github.com/Microsoft/hcsshim/internal/guest/prot"
  "github.com/Microsoft/hcsshim/internal/guest/runtime/hcsv2"
  "github.com/Microsoft/hcsshim/internal/log"
  "github.com/Microsoft/hcsshim/internal/oc"
- "github.com/pkg/errors"
- "github.com/sirupsen/logrus"
- "go.opencensus.io/trace"
- "go.opencensus.io/trace/tracestate"
 )
 
 // UnknownMessage represents the default handler logic for an unmatched request
@@ -291,16 +292,18 @@ func (b *Bridge) ListenAndServe(bridgeIn io.ReadCloser, bridgeOut io.WriteCloser
  }
  }
  }
- ctx, span = trace.StartSpanWithRemoteParent(context.Background(),
+ ctx, span = oc.StartSpanWithRemoteParent(
+ context.Background(),
  "opengcs::bridge::request",
  sc,
- oc.WithTraceLevelSampler,
- oc.WithServerSpanKind)
- ctx = log.U(ctx)
+ oc.WithServerSpanKind,
+ )
  } else {
- ctx, span = oc.StartSpan(context.Background(),
+ ctx, span = oc.StartSpan(
+ context.Background(),
  "opengcs::bridge::request",
- oc.WithServerSpanKind)
+ oc.WithServerSpanKind,
+ )
  }
 
  span.AddAttributes(

internal/guest/network/network.go

@@ -13,13 +13,13 @@ import (
  "strings"
  "time"
 
+ "github.com/pkg/errors"
+ "github.com/sirupsen/logrus"
+
  "github.com/Microsoft/hcsshim/internal/guest/storage"
  "github.com/Microsoft/hcsshim/internal/guest/storage/pci"
  "github.com/Microsoft/hcsshim/internal/guest/storage/vmbus"
  "github.com/Microsoft/hcsshim/internal/log"
- "github.com/Microsoft/hcsshim/internal/oc"
- "github.com/pkg/errors"
- "go.opencensus.io/trace"
 )
 
 // mock out calls for testing
@@ -35,9 +35,9 @@ const maxDNSSearches = 6
 
 // GenerateEtcHostsContent generates a /etc/hosts file based on `hostname`.
 func GenerateEtcHostsContent(ctx context.Context, hostname string) string {
- _, span := oc.StartSpan(ctx, "network::GenerateEtcHostsContent")
- defer span.End()
- span.AddAttributes(trace.StringAttribute("hostname", hostname))
+ log.G(ctx).WithFields(logrus.Fields{
+  "hostname": hostname,
+ }).Trace("network::GenerateEtcHostsContent")
 
  nameParts := strings.Split(hostname, ".")
  buf := bytes.Buffer{}
@@ -60,14 +60,11 @@ func GenerateEtcHostsContent(ctx context.Context, hostname string) string {
 // GenerateResolvConfContent generates the resolv.conf file content based on
 // `searches`, `servers`, and `options`.
 func GenerateResolvConfContent(ctx context.Context, searches, servers, options []string) (_ string, err error) {
- _, span := oc.StartSpan(ctx, "network::GenerateResolvConfContent")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
-
- span.AddAttributes(
- trace.StringAttribute("searches", strings.Join(searches, ", ")),
- trace.StringAttribute("servers", strings.Join(servers, ", ")),
- trace.StringAttribute("options", strings.Join(options, ", ")))
+ log.G(ctx).WithFields(logrus.Fields{
+ "searches": searches,
+ "servers": servers,
+ "options": options,
+ }).Trace("network::GenerateResolvConfContent")
 
  if len(searches) > maxDNSSearches {
  return "", errors.Errorf("searches has more than %d domains", maxDNSSearches)
@@ -116,12 +113,10 @@ func MergeValues(first, second []string) []string {
 //
 // Will retry the operation until `ctx` is exceeded or canceled.
 func InstanceIDToName(ctx context.Context, id string, vpciAssigned bool) (_ string, err error) {
- ctx, span := oc.StartSpan(ctx, "network::InstanceIDToName")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
-
  vmBusID := strings.ToLower(id)
- span.AddAttributes(trace.StringAttribute("adapterInstanceID", vmBusID))
+ log.G(ctx).WithFields(logrus.Fields{
+ "adapterInstanceID": vmBusID,
+ }).Trace("network::InstanceIDToName")
 
  netDevicePath := ""
  if vpciAssigned {

internal/guest/runtime/hcsv2/container.go

@@ -13,7 +13,6 @@ import (
  oci "github.com/opencontainers/runtime-spec/specs-go"
  "github.com/pkg/errors"
  "github.com/sirupsen/logrus"
- "go.opencensus.io/trace"
 
  "github.com/Microsoft/hcsshim/internal/guest/gcserr"
  "github.com/Microsoft/hcsshim/internal/guest/prot"
@@ -24,7 +23,6 @@ import (
  "github.com/Microsoft/hcsshim/internal/guest/transport"
  "github.com/Microsoft/hcsshim/internal/log"
  "github.com/Microsoft/hcsshim/internal/logfields"
- "github.com/Microsoft/hcsshim/internal/oc"
  "github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
  "github.com/Microsoft/hcsshim/internal/protocol/guestresource"
 )
@@ -47,7 +45,7 @@ type Container struct {
 }
 
 func (c *Container) Start(ctx context.Context, conSettings stdio.ConnectionSettings) (int, error) {
- log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::Start")
+ log.G(ctx).WithField(logfields.ContainerID, c.id).Trace("opengcs::Container::Start")
  stdioSet, err := stdio.Connect(c.vsock, conSettings)
  if err != nil {
  return -1, err
@@ -70,7 +68,7 @@ func (c *Container) Start(ctx context.Context, conSettings stdio.ConnectionSetti
 }
 
 func (c *Container) ExecProcess(ctx context.Context, process *oci.Process, conSettings stdio.ConnectionSettings) (int, error) {
- log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::ExecProcess")
+ log.G(ctx).WithField(logfields.ContainerID, c.id).Trace("opengcs::Container::ExecProcess")
  stdioSet, err := stdio.Connect(c.vsock, conSettings)
  if err != nil {
  return -1, err
@@ -115,7 +113,7 @@ func (c *Container) GetProcess(pid uint32) (Process, error) {
  logrus.WithFields(logrus.Fields{
  logfields.ContainerID: c.id,
  logfields.ProcessID: pid,
- }).Info("opengcs::Container::GetProcesss")
+ }).Trace("opengcs::Container::GetProcesss")
  if c.initProcess.pid == pid {
  return c.initProcess, nil
  }
@@ -132,7 +130,7 @@ func (c *Container) GetProcess(pid uint32) (Process, error) {
 
 // GetAllProcessPids returns all process pids in the container namespace.
 func (c *Container) GetAllProcessPids(ctx context.Context) ([]int, error) {
- log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::GetAllProcessPids")
+ log.G(ctx).WithField(logfields.ContainerID, c.id).Trace("opengcs::Container::GetAllProcessPids")
  state, err := c.container.GetAllProcesses()
  if err != nil {
  return nil, err
@@ -146,7 +144,7 @@ func (c *Container) GetAllProcessPids(ctx context.Context) ([]int, error) {
 
 // Kill sends 'signal' to the container process.
 func (c *Container) Kill(ctx context.Context, signal syscall.Signal) error {
- log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::Kill")
+ log.G(ctx).WithField(logfields.ContainerID, c.id).Trace("opengcs::Container::Kill")
  err := c.container.Kill(signal)
  if err != nil {
  return err
@@ -157,7 +155,7 @@ func (c *Container) Kill(ctx context.Context, signal syscall.Signal) error {
 
 func (c *Container) Delete(ctx context.Context) error {
  entity := log.G(ctx).WithField(logfields.ContainerID, c.id)
- entity.Info("opengcs::Container::Delete")
+ entity.Trace("opengcs::Container::Delete")
  if c.isSandbox {
  // remove user mounts in sandbox container
  if err := storage.UnmountAllInPath(ctx, specInternal.SandboxMountsDir(c.id), true); err != nil {
@@ -174,15 +172,13 @@ func (c *Container) Delete(ctx context.Context) error {
 }
 
 func (c *Container) Update(ctx context.Context, resources interface{}) error {
- log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::Update")
+ log.G(ctx).WithField(logfields.ContainerID, c.id).Trace("opengcs::Container::Update")
  return c.container.Update(resources)
 }
 
 // Wait waits for the container's init process to exit.
 func (c *Container) Wait() prot.NotificationType {
- _, span := oc.StartSpan(context.Background(), "opengcs::Container::Wait")
- defer span.End()
- span.AddAttributes(trace.StringAttribute(logfields.ContainerID, c.id))
+ log.L.WithField(logfields.ContainerID, c.id).Trace("opengcs::Container::Wait")
 
  c.initProcess.writersWg.Wait()
  c.etL.Lock()
@@ -205,9 +201,7 @@ func (c *Container) setExitType(signal syscall.Signal) {
 
 // GetStats returns the cgroup metrics for the container.
 func (c *Container) GetStats(ctx context.Context) (*v1.Metrics, error) {
- _, span := oc.StartSpan(ctx, "opengcs::Container::GetStats")
- defer span.End()
- span.AddAttributes(trace.StringAttribute("cid", c.id))
+ log.G(ctx).WithField(logfields.ContainerID, c.id).Trace("opengcs::Container::GetStats")
 
  cgroupPath := c.spec.Linux.CgroupsPath
  cg, err := cgroups.Load(cgroups.V1, cgroups.StaticPath(cgroupPath))

internal/guest/runtime/hcsv2/network.go

@@ -5,19 +5,19 @@ package hcsv2
 
 import (
  "context"
- "fmt"
  "strings"
  "sync"
  "time"
 
  "github.com/pkg/errors"
+ "github.com/sirupsen/logrus"
  "github.com/vishvananda/netns"
- "go.opencensus.io/trace"
 
  "github.com/Microsoft/hcsshim/internal/guest/gcserr"
  "github.com/Microsoft/hcsshim/internal/guest/network"
  "github.com/Microsoft/hcsshim/internal/guest/prot"
- "github.com/Microsoft/hcsshim/internal/oc"
+ "github.com/Microsoft/hcsshim/internal/log"
+ "github.com/Microsoft/hcsshim/internal/logfields"
  "github.com/Microsoft/hcsshim/internal/protocol/guestresource"
 )
 
@@ -71,12 +71,8 @@ func getOrAddNetworkNamespace(id string) *namespace {
 
 // removeNetworkNamespace removes the in-memory `namespace` found by `id`.
 func removeNetworkNamespace(ctx context.Context, id string) (err error) {
- _, span := oc.StartSpan(ctx, "hcsv2::removeNetworkNamespace")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
-
  id = strings.ToLower(id)
- span.AddAttributes(trace.StringAttribute("id", id))
+ log.G(ctx).WithField(logfields.ID, id).Trace("hcsv2::removeNetworkNamespace")
 
  namespaceSync.Lock()
  defer namespaceSync.Unlock()
@@ -112,12 +108,10 @@ func (n *namespace) ID() string {
 // assigned adapters into this namespace. The caller MUST call `Sync()` to
 // complete this operation.
 func (n *namespace) AssignContainerPid(ctx context.Context, pid int) (err error) {
- _, span := oc.StartSpan(ctx, "namespace::AssignContainerPid")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
- span.AddAttributes(
- trace.StringAttribute("namespace", n.id),
- trace.Int64Attribute("pid", int64(pid)))
+ log.G(ctx).WithFields(logrus.Fields{
+ logfields.ProcessID: pid,
+ logfields.Namespace: n.id,
+ }).Trace("namespace::AssignContainerPid")
 
  n.m.Lock()
  defer n.m.Unlock()
@@ -147,12 +141,10 @@ func (n *namespace) Adapters() []*guestresource.LCOWNetworkAdapter {
 // namespace assigned to `n`. A user must call `Sync()` to complete this
 // operation.
 func (n *namespace) AddAdapter(ctx context.Context, adp *guestresource.LCOWNetworkAdapter) (err error) {
- ctx, span := oc.StartSpan(ctx, "namespace::AddAdapter")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
- span.AddAttributes(
- trace.StringAttribute("namespace", n.id),
- trace.StringAttribute("adapter", log.Format(ctx, adp)))
+ log.G(ctx).WithFields(logrus.Fields{
+ logfields.Namespace: n.id,
+ "adapter": adp,
+ }).Trace("namespace::AddAdapter")
 
  n.m.Lock()
  defer n.m.Unlock()
@@ -179,12 +171,10 @@ func (n *namespace) AddAdapter(ctx context.Context, adp *guestresource.LCOWNetwo
 // RemoveAdapter removes the adapter matching `id` from `n`. If `id` is not
 // found returns no error.
 func (n *namespace) RemoveAdapter(ctx context.Context, id string) (err error) {
- _, span := oc.StartSpan(ctx, "namespace::RemoveAdapter")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
- span.AddAttributes(
- trace.StringAttribute("namespace", n.id),
- trace.StringAttribute("adapterID", id))
+ log.G(ctx).WithFields(logrus.Fields{
+ logfields.Namespace: n.id,
+ "adapterID": id,
+ }).Trace("namespace::RemoveAdapter")
 
  n.m.Lock()
  defer n.m.Unlock()
@@ -206,10 +196,7 @@ func (n *namespace) RemoveAdapter(ctx context.Context, id string) (err error) {
 
 // Sync moves all adapters to the network namespace of `n` if assigned.
 func (n *namespace) Sync(ctx context.Context) (err error) {
- ctx, span := oc.StartSpan(ctx, "namespace::Sync")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
- span.AddAttributes(trace.StringAttribute("namespace", n.id))
+ log.G(ctx).WithField(logfields.Namespace, n.id).Trace("namespace::Sync")
 
  n.m.Lock()
  defer n.m.Unlock()
@@ -244,13 +231,11 @@ type nicInNamespace struct {
 
 // assignToPid assigns `nin.adapter`, represented by `nin.ifname` to `pid`.
 func (nin *nicInNamespace) assignToPid(ctx context.Context, pid int) (err error) {
- ctx, span := oc.StartSpan(ctx, "nicInNamespace::assignToPid")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
- span.AddAttributes(
- trace.StringAttribute("adapterID", nin.adapter.ID),
- trace.StringAttribute("ifname", nin.ifname),
- trace.Int64Attribute("pid", int64(pid)))
+ log.G(ctx).WithFields(logrus.Fields{
+ "adapterID": nin.adapter.ID,
+ "ifname": nin.ifname,
+ logfields.ProcessID: int64(pid),
+ }).Trace("nicInNamespace::assignToPid")
 
  v1Adapter := &prot.NetworkAdapter{
  NatEnabled: nin.adapter.IPAddress != "",

internal/guest/runtime/hcsv2/process.go

@@ -14,6 +14,7 @@ import (
  "github.com/Microsoft/hcsshim/internal/guest/runtime"
  "github.com/Microsoft/hcsshim/internal/guest/stdio"
  "github.com/Microsoft/hcsshim/internal/log"
+ "github.com/Microsoft/hcsshim/internal/logfields"
  "github.com/Microsoft/hcsshim/internal/oc"
  oci "github.com/opencontainers/runtime-spec/specs-go"
  "github.com/pkg/errors"
@@ -87,8 +88,8 @@ func newProcess(c *Container, spec *oci.Process, process runtime.Process, pid ui
  ctx, span := oc.StartSpan(context.Background(), "newProcess::waitBackground")
  defer span.End()
  span.AddAttributes(
- trace.StringAttribute("cid", p.cid),
- trace.Int64Attribute("pid", int64(p.pid)))
+ trace.StringAttribute(logfields.ContainerID, p.cid),
+ trace.Int64Attribute(logfields.ProcessID, int64(p.pid)))
 
  // Wait for the process to exit
  exitCode, err := p.process.Wait()

internal/guest/runtime/hcsv2/sandbox_container.go

@@ -12,11 +12,11 @@ import (
 
  oci "github.com/opencontainers/runtime-spec/specs-go"
  "github.com/pkg/errors"
- "go.opencensus.io/trace"
 
  "github.com/Microsoft/hcsshim/internal/guest/network"
  specInternal "github.com/Microsoft/hcsshim/internal/guest/spec"
- "github.com/Microsoft/hcsshim/internal/oc"
+ "github.com/Microsoft/hcsshim/internal/log"
+ "github.com/Microsoft/hcsshim/internal/logfields"
  "github.com/Microsoft/hcsshim/pkg/annotations"
 )
 
@@ -33,10 +33,7 @@ func getSandboxResolvPath(id string) string {
 }
 
 func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (err error) {
- ctx, span := oc.StartSpan(ctx, "hcsv2::setupSandboxContainerSpec")
- defer span.End()
- defer func() { oc.SetSpanStatus(span, err) }()
- span.AddAttributes(trace.StringAttribute("cid", id))
+ log.G(ctx).WithField(logfields.ContainerID, id).Trace("hcsv2::setupSandboxContainerSpec")
 
  // Generate the sandbox root dir
  rootDir := specInternal.SandboxRootDir(id)