From 0b67db7cbaeed194d2fe64c77f41cb5d5bb29a77 Mon Sep 17 00:00:00 2001 From: Daniel Grau Date: Mon, 16 Sep 2024 13:09:11 -0700 Subject: [PATCH] Migrate saiserver to slog (#477) * Migrate saiserver to slog * lint --- dataplane/saiserver/BUILD | 1 - dataplane/saiserver/attrmgr/BUILD | 1 - dataplane/saiserver/attrmgr/attrmgr.go | 8 ++++---- dataplane/saiserver/hostif.go | 23 +++++++++++------------ dataplane/saiserver/ports.go | 9 ++++----- dataplane/saiserver/routing.go | 11 +++++------ dataplane/saiserver/saiserver.go | 9 ++++----- dataplane/saiserver/switch.go | 9 ++++----- 8 files changed, 32 insertions(+), 39 deletions(-) diff --git a/dataplane/saiserver/BUILD b/dataplane/saiserver/BUILD index d0b00ab6..1eb0aee3 100644 --- a/dataplane/saiserver/BUILD +++ b/dataplane/saiserver/BUILD @@ -27,7 +27,6 @@ go_library( "//dataplane/proto/sai", "//dataplane/saiserver/attrmgr", "//proto/forwarding", - "@com_github_golang_glog//:glog", "@com_github_openconfig_gnmi//errlist", "@io_opentelemetry_go_otel//:otel", "@io_opentelemetry_go_otel_trace//:trace", diff --git a/dataplane/saiserver/attrmgr/BUILD b/dataplane/saiserver/attrmgr/BUILD index ecfa3ead..6d31385f 100644 --- a/dataplane/saiserver/attrmgr/BUILD +++ b/dataplane/saiserver/attrmgr/BUILD @@ -7,7 +7,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//dataplane/proto/sai", - "@com_github_golang_glog//:glog", "@org_golang_google_grpc//:go_default_library", "@org_golang_google_grpc//codes", "@org_golang_google_grpc//status", diff --git a/dataplane/saiserver/attrmgr/attrmgr.go b/dataplane/saiserver/attrmgr/attrmgr.go index 6a85e0c9..8140ceba 100644 --- a/dataplane/saiserver/attrmgr/attrmgr.go +++ b/dataplane/saiserver/attrmgr/attrmgr.go @@ -20,12 +20,12 @@ package attrmgr import ( "context" "fmt" + "log/slog" "reflect" "strings" "sync" "sync/atomic" - log "github.com/golang/glog" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -93,7 +93,7 @@ func InvokeAndSave[T proto.Message, S proto.Message](ctx context.Context, mgr *A } id, err := mgr.getID(req, respMsg) if err != nil { - log.Warningf("failed to get id %v", err) + slog.WarnContext(ctx, "failed to get id", "err", err) return respMsg.(S), nil } mgr.storeAttributes(id, req) @@ -124,7 +124,7 @@ func (mgr *AttrMgr) Interceptor(ctx context.Context, req any, info *grpc.UnarySe case strings.Contains(info.FullMethod, "Create") || strings.Contains(info.FullMethod, "Set"): id, err := mgr.getID(reqMsg, respMsg) if err != nil { - log.Warningf("failed to get id %v", err) + slog.WarnContext(ctx, "failed to get id", "err", err) return respMsg, nil } mgr.storeAttributes(id, reqMsg) @@ -135,7 +135,7 @@ func (mgr *AttrMgr) Interceptor(ctx context.Context, req any, info *grpc.UnarySe case strings.Contains(info.FullMethod, "Remove"): id, err := mgr.getID(reqMsg, respMsg) if err != nil { - log.Warningf("failed to get id %v", err) + slog.WarnContext(ctx, "failed to get id", "err", err) return respMsg, nil } if err := deleteOID(mgr, id); err != nil { diff --git a/dataplane/saiserver/hostif.go b/dataplane/saiserver/hostif.go index 547e007a..a86d7a09 100644 --- a/dataplane/saiserver/hostif.go +++ b/dataplane/saiserver/hostif.go @@ -17,6 +17,7 @@ package saiserver import ( "context" "fmt" + "log/slog" "strings" "sync" @@ -30,8 +31,6 @@ import ( "github.com/openconfig/lemming/dataplane/forwarding/fwdport/ports" "github.com/openconfig/lemming/dataplane/saiserver/attrmgr" - log "github.com/golang/glog" - pktiopb "github.com/openconfig/lemming/dataplane/proto/packetio" saipb "github.com/openconfig/lemming/dataplane/proto/sai" fwdpb "github.com/openconfig/lemming/proto/forwarding" @@ -66,7 +65,7 @@ type hostif struct { } func (hostif *hostif) Reset() { - log.Info("resetting hostif") + slog.Info("resetting hostif") for _, closeFn := range hostif.remoteClosers { closeFn() } @@ -401,14 +400,14 @@ func (hostif *hostif) CPUPacketStream(srv pktiopb.PacketIO_CPUPacketStreamServer case <-ctx.Done(): return nil case pkt := <-packetCh: - log.V(3).Infof("received packet %x", pkt.GetPacket().GetFrame()) + slog.Debug("received packet", "packet", pkt.GetPacket().GetFrame()) acts := []*fwdpb.ActionDesc{fwdconfig.Action(fwdconfig.UpdateAction(fwdpb.UpdateType_UPDATE_TYPE_SET, fwdpb.PacketFieldNum_PACKET_FIELD_NUM_HOST_PORT_ID). WithUint64Value(pkt.GetPacket().GetHostPort())).Build()} err = hostif.dataplane.InjectPacket(&fwdpb.ContextId{Id: hostif.dataplane.ID()}, &fwdpb.PortId{ObjectId: &fwdpb.ObjectId{Id: cpuPortID}}, fwdpb.PacketHeaderId_PACKET_HEADER_ID_ETHERNET, pkt.GetPacket().GetFrame(), acts, true, fwdpb.PortAction_PORT_ACTION_INPUT) if err != nil { - log.Warningf("inject err: %v", err) + slog.WarnContext(ctx, "inject err", "err", err) continue } } @@ -416,17 +415,17 @@ func (hostif *hostif) CPUPacketStream(srv pktiopb.PacketIO_CPUPacketStreamServer } func (hostif *hostif) HostPortControl(srv pktiopb.PacketIO_HostPortControlServer) error { - log.Info("started host port control channel") + slog.InfoContext(srv.Context(), "started host port control channel") _, err := srv.Recv() if err != nil { return err } - log.Info("received init port control channel") + slog.InfoContext(srv.Context(), "received init port control channel") hostif.remoteMu.Lock() ctx, cancelFn := context.WithCancel(srv.Context()) hostif.remoteClosers = append(hostif.remoteClosers, func() { - log.Info("canceling host port control") + slog.InfoContext(srv.Context(), "canceling host port control") cancelFn() }) @@ -454,20 +453,20 @@ func (hostif *hostif) HostPortControl(srv pktiopb.PacketIO_HostPortControlServer } hostif.remoteMu.Unlock() - log.Info("initialized host port control channel") + slog.InfoContext(srv.Context(), "initialized host port control channel") // The HostPortControls exits in two cases: context cancels or RPC errors. err = nil select { case <-ctx.Done(): - log.Info("host port control done") + slog.InfoContext(srv.Context(), "host port control done") case err = <-errCh: - log.Info("host port control err: %v", err) + slog.InfoContext(srv.Context(), "host port control err", "err", err) } hostif.remoteMu.Lock() hostif.remotePortReq = nil hostif.remoteMu.Unlock() - log.Info("cleared host port control channel") + slog.InfoContext(srv.Context(), "cleared host port control channel") return err } diff --git a/dataplane/saiserver/ports.go b/dataplane/saiserver/ports.go index e7b7e9aa..1c4cedc5 100644 --- a/dataplane/saiserver/ports.go +++ b/dataplane/saiserver/ports.go @@ -17,6 +17,7 @@ package saiserver import ( "context" "fmt" + "log/slog" "net" "google.golang.org/grpc" @@ -26,8 +27,6 @@ import ( "github.com/openconfig/lemming/dataplane/forwarding/fwdconfig" "github.com/openconfig/lemming/dataplane/saiserver/attrmgr" - log "github.com/golang/glog" - saipb "github.com/openconfig/lemming/dataplane/proto/sai" fwdpb "github.com/openconfig/lemming/proto/forwarding" ) @@ -281,7 +280,7 @@ func (port *port) CreatePort(ctx context.Context, req *saipb.CreatePortRequest) } fwdPort.Port.PortType = port.opts.PortType - log.Infof("created port %v, dev %v with lanes %v", id, dev, req.GetHwLaneList()) + slog.InfoContext(ctx, "created port", "port", id, "device", dev, "hwlane", req.GetHwLaneList()) _, err := port.dataplane.PortCreate(ctx, fwdPort) if err != nil { return nil, err @@ -324,7 +323,7 @@ func (port *port) CreatePort(ctx context.Context, req *saipb.CreatePortRequest) ObjectId: &fwdpb.ObjectId{Id: fmt.Sprint(id)}, }) if err != nil { - log.Infof("Failed to find NID for port id=%d: %v", id, err) + slog.InfoContext(ctx, "Failed to find NID for port", "port", id, "err", err) return nil, err } vlanReq := fwdconfig.TableEntryAddRequest(port.dataplane.ID(), VlanTable).AppendEntry( @@ -568,7 +567,7 @@ func (port *port) RemovePort(ctx context.Context, req *saipb.RemovePortRequest) } func (port *port) Reset() { - log.Info("reseting port") + slog.Info("reseting port") } type lagMember struct { diff --git a/dataplane/saiserver/routing.go b/dataplane/saiserver/routing.go index 924d31e7..6ad96048 100644 --- a/dataplane/saiserver/routing.go +++ b/dataplane/saiserver/routing.go @@ -18,6 +18,7 @@ import ( "context" "encoding/binary" "fmt" + "log/slog" "sync" "google.golang.org/grpc" @@ -30,8 +31,6 @@ import ( "github.com/openconfig/lemming/dataplane/forwarding/fwdconfig" "github.com/openconfig/lemming/dataplane/saiserver/attrmgr" - log "github.com/golang/glog" - saipb "github.com/openconfig/lemming/dataplane/proto/sai" fwdpb "github.com/openconfig/lemming/proto/forwarding" ) @@ -674,7 +673,7 @@ func (ri *routerInterface) CreateRouterInterface(ctx context.Context, req *saipb switch req.GetType() { case saipb.RouterInterfaceType_ROUTER_INTERFACE_TYPE_PORT: case saipb.RouterInterfaceType_ROUTER_INTERFACE_TYPE_LOOPBACK: // TODO: Support loopback interfaces - log.Warning("loopback interfaces not supported") + slog.WarnContext(ctx, "loopback interfaces not supported") return &saipb.CreateRouterInterfaceResponse{Oid: id}, nil default: return nil, status.Errorf(codes.InvalidArgument, "unknown interface type: %v", req.GetType()) @@ -970,7 +969,7 @@ func (vlan *vlan) CreateVlanMember(ctx context.Context, r *saipb.CreateVlanMembe ObjectId: &fwdpb.ObjectId{Id: fmt.Sprint(r.GetBridgePortId())}, }) if err != nil { - log.Infof("Failed to find NID for port id=%d: %v", r.GetBridgePortId(), err) + slog.InfoContext(ctx, "Failed to find NID for port", "bridge_port", r.GetBridgePortId(), "err", err) return nil, err } vlanReq := fwdconfig.TableEntryAddRequest(vlan.dataplane.ID(), VlanTable).AppendEntry( @@ -1026,7 +1025,7 @@ func (vlan *vlan) RemoveVlanMember(ctx context.Context, r *saipb.RemoveVlanMembe ObjectId: &fwdpb.ObjectId{Id: fmt.Sprint(member.PortID)}, }) if err != nil { - log.Infof("Failed to find NID for port id=%d: %v", member.PortID, err) + slog.InfoContext(ctx, "Failed to find NID for port", "bridge_port", member.PortID, "err", err) return nil, err } if _, err := vlan.dataplane.TableEntryRemove(ctx, fwdconfig.TableEntryRemoveRequest(vlan.dataplane.ID(), VlanTable).AppendEntry( @@ -1049,7 +1048,7 @@ func (vlan *vlan) CreateVlanMembers(ctx context.Context, r *saipb.CreateVlanMemb } func (vlan *vlan) Reset() { - log.Info("resetting vlan") + slog.Info("resetting vlan") vlan.oidByVId = map[uint32]uint64{} vlan.vlans = map[uint64]map[uint64]*vlanMember{} } diff --git a/dataplane/saiserver/saiserver.go b/dataplane/saiserver/saiserver.go index 886a97b5..f443c225 100644 --- a/dataplane/saiserver/saiserver.go +++ b/dataplane/saiserver/saiserver.go @@ -17,6 +17,7 @@ package saiserver import ( "context" "fmt" + "log/slog" "github.com/openconfig/lemming/dataplane/dplaneopts" "github.com/openconfig/lemming/dataplane/forwarding" @@ -24,8 +25,6 @@ import ( "google.golang.org/grpc" - log "github.com/golang/glog" - saipb "github.com/openconfig/lemming/dataplane/proto/sai" fwdpb "github.com/openconfig/lemming/proto/forwarding" ) @@ -136,10 +135,10 @@ type Server struct { tam *tam } -func (s *Server) ObjectTypeQuery(_ context.Context, req *saipb.ObjectTypeQueryRequest) (*saipb.ObjectTypeQueryResponse, error) { +func (s *Server) ObjectTypeQuery(ctx context.Context, req *saipb.ObjectTypeQueryRequest) (*saipb.ObjectTypeQueryResponse, error) { val := s.mgr.GetType(fmt.Sprint(req.GetObject())) if val == saipb.ObjectType_OBJECT_TYPE_NULL { - log.Warningf("unknown object id %v, type %v", req.Object, val) + slog.WarnContext(ctx, "unknown object id", "oid", req.Object) } return &saipb.ObjectTypeQueryResponse{ Type: val, @@ -148,7 +147,7 @@ func (s *Server) ObjectTypeQuery(_ context.Context, req *saipb.ObjectTypeQueryRe func (s *Server) Initialize(ctx context.Context, _ *saipb.InitializeRequest) (*saipb.InitializeResponse, error) { if s.initialized { - log.Info("dataplane already intialized, reseting") + slog.InfoContext(ctx, "dataplane already intialized, reseting") s.mgr.Reset() s.saiSwitch.Reset() if err := s.Reset(ctx); err != nil { diff --git a/dataplane/saiserver/switch.go b/dataplane/saiserver/switch.go index 1c4da54e..7aafd19f 100644 --- a/dataplane/saiserver/switch.go +++ b/dataplane/saiserver/switch.go @@ -17,6 +17,7 @@ package saiserver import ( "context" "fmt" + "log/slog" "net" "strconv" @@ -25,8 +26,6 @@ import ( "google.golang.org/grpc" "google.golang.org/protobuf/proto" - log "github.com/golang/glog" - "github.com/openconfig/lemming/dataplane/dplaneopts" "github.com/openconfig/lemming/dataplane/forwarding/fwdconfig" "github.com/openconfig/lemming/dataplane/forwarding/infra/fwdcontext" @@ -969,7 +968,7 @@ func (sw *saiSwitch) PortStateChangeNotification(_ *saipb.PortStateChangeNotific case ed := <-fwdSrv.ch: num, err := strconv.Atoi(ed.GetPort().GetPortId().GetObjectId().GetId()) if err != nil { - log.Warningf("couldn't get numeric port id: %v", err) + slog.WarnContext(srv.Context(), "couldn't get numeric port id", "err", err) continue } oType := sw.mgr.GetType(ed.GetPort().GetPortId().GetObjectId().GetId()) @@ -978,7 +977,7 @@ func (sw *saiSwitch) PortStateChangeNotification(_ *saipb.PortStateChangeNotific case saipb.ObjectType_OBJECT_TYPE_BRIDGE_PORT: case saipb.ObjectType_OBJECT_TYPE_LAG: default: - log.Infof("skipping port state event for type %v", oType) + slog.InfoContext(srv.Context(), "skipping port state event", "type", oType) continue } status := saipb.PortOperStatus_PORT_OPER_STATUS_UNKNOWN @@ -993,7 +992,7 @@ func (sw *saiSwitch) PortStateChangeNotification(_ *saipb.PortStateChangeNotific PortState: status, }}, } - log.Infof("send port event: %+v", resp) + slog.InfoContext(srv.Context(), "send port event", "event", resp) err = srv.Send(resp) if err != nil { return err