Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrate assume_cache.go to structured logging #105904

Merged
merged 1 commit into from
Nov 9, 2021
Merged

Migrate assume_cache.go to structured logging #105904

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 14 additions & 14 deletions pkg/scheduler/framework/plugins/volumebinding/assume_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (c *assumeCache) add(obj interface{}) {

name, err := cache.MetaNamespaceKeyFunc(obj)
if err != nil {
klog.Errorf("add failed: %v", &errObjectName{err})
klog.ErrorS(&errObjectName{err}, "Add failed")
return
}

Expand All @@ -171,29 +171,29 @@ func (c *assumeCache) add(obj interface{}) {
if objInfo, _ := c.getObjInfo(name); objInfo != nil {
newVersion, err := c.getObjVersion(name, obj)
if err != nil {
klog.Errorf("add: couldn't get object version: %v", err)
klog.ErrorS(err, "Add failed: couldn't get object version")
return
}

storedVersion, err := c.getObjVersion(name, objInfo.latestObj)
if err != nil {
klog.Errorf("add: couldn't get stored object version: %v", err)
klog.ErrorS(err, "Add failed: couldn't get stored object version")
return
}

// Only update object if version is newer.
// This is so we don't override assumed objects due to informer resync.
if newVersion <= storedVersion {
klog.V(10).Infof("Skip adding %v %v to assume cache because version %v is not newer than %v", c.description, name, newVersion, storedVersion)
klog.V(10).InfoS("Skip adding object to assume cache because version is not newer than storedVersion", "description", c.description, "cacheKey", name, "newVersion", newVersion, "storedVersion", storedVersion)
return
}
}

objInfo := &objInfo{name: name, latestObj: obj, apiObj: obj}
if err = c.store.Update(objInfo); err != nil {
klog.Warningf("got error when updating stored object : %v", err)
klog.InfoS("Error occurred while updating stored object", "err", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does structured logging use Info to replace Warning msgs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the stuctured-logging-migration-guide, the concept behind using info is explained in logr, quoting verbatim for quick reference

Info logs are things you want to tell the user which are not errors. Error logs are, well, errors. If your code receives an error from a subordinate function call and is logging that error and not returning it, use error logs.

Verbosity-levels on info logs. This gives developers a chance to indicate arbitrary grades of importance for info logs, without assigning names with semantic meaning such as "warning", "trace", and "debug." Superficially this may feel very similar, but the primary difference is the lack of semantics. Because verbosity is a numerical value, it's safe to assume that an app running with higher verbosity means more (and less important) logs will be generated.

Hope, this helps :)

} else {
klog.V(10).Infof("Adding %v %v to assume cache: %+v ", c.description, name, obj)
klog.V(10).InfoS("Adding object to assume cache", "description", c.description, "cacheKey", name, "assumeCache", obj)
}
}

Expand All @@ -208,7 +208,7 @@ func (c *assumeCache) delete(obj interface{}) {

name, err := cache.MetaNamespaceKeyFunc(obj)
if err != nil {
klog.Errorf("delete failed: %v", &errObjectName{err})
klog.ErrorS(&errObjectName{err}, "Failed to delete")
return
}

Expand All @@ -218,7 +218,7 @@ func (c *assumeCache) delete(obj interface{}) {
objInfo := &objInfo{name: name}
err = c.store.Delete(objInfo)
if err != nil {
klog.Errorf("delete: failed to delete %v %v: %v", c.description, name, err)
klog.ErrorS(err, "Failed to delete", "description", c.description, "cacheKey", name)
}
}

Expand Down Expand Up @@ -280,14 +280,14 @@ func (c *assumeCache) List(indexObj interface{}) []interface{} {
allObjs := []interface{}{}
objs, err := c.store.Index(c.indexName, &objInfo{latestObj: indexObj})
if err != nil {
klog.Errorf("list index error: %v", err)
klog.ErrorS(err, "List index error")
return nil
}

for _, obj := range objs {
objInfo, ok := obj.(*objInfo)
if !ok {
klog.Errorf("list error: %v", &errWrongType{"objInfo", obj})
klog.ErrorS(&errWrongType{"objInfo", obj}, "List error")
continue
}
allObjs = append(allObjs, objInfo.latestObj)
Expand Down Expand Up @@ -325,7 +325,7 @@ func (c *assumeCache) Assume(obj interface{}) error {

// Only update the cached object
objInfo.latestObj = obj
klog.V(4).Infof("Assumed %v %q, version %v", c.description, name, newVersion)
klog.V(4).InfoS("Assumed object", "description", c.description, "cacheKey", name, "version", newVersion)
return nil
}

Expand All @@ -336,10 +336,10 @@ func (c *assumeCache) Restore(objName string) {
objInfo, err := c.getObjInfo(objName)
if err != nil {
// This could be expected if object got deleted
klog.V(5).Infof("Restore %v %q warning: %v", c.description, objName, err)
klog.V(5).InfoS("Restore object", "description", c.description, "cacheKey", objName, "err", err)
} else {
objInfo.latestObj = objInfo.apiObj
klog.V(4).Infof("Restored %v %q", c.description, objName)
klog.V(4).InfoS("Restored object", "description", c.description, "cacheKey", objName)
}
}

Expand Down Expand Up @@ -403,7 +403,7 @@ func (c *pvAssumeCache) ListPVs(storageClassName string) []*v1.PersistentVolume
for _, obj := range objs {
pv, ok := obj.(*v1.PersistentVolume)
if !ok {
klog.Errorf("ListPVs: %v", &errWrongType{"v1.PersistentVolume", obj})
klog.ErrorS(&errWrongType{"v1.PersistentVolume", obj}, "ListPVs")
mengjiao-liu marked this conversation as resolved.
Show resolved Hide resolved
continue
}
pvs = append(pvs, pv)
Expand Down