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.

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

Added logs to cluster extraction #428

Merged
merged 1 commit into from
Mar 28, 2023
Merged

Added logs to cluster extraction #428

merged 1 commit into from
Mar 28, 2023

Conversation

rmweir
Copy link
Contributor

@rmweir rmweir commented Mar 24, 2023

No description provided.

@rmweir rmweir requested review from cbron and KevinJoiner March 24, 2023 19:37
Copy link
Contributor

@KevinJoiner KevinJoiner left a comment

Choose a reason for hiding this comment

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

Is the desire to leave this in forever? I am concerned that there are healthy Rancher setups that will start spamming this message since we are not sure what causes it. Or can we assume that anytime clusterName is empty, something is not correct?

ns := getValue(obj, "Namespace")
name := getValue(obj, "Name")
kind := getValue(obj, "Kind")
logrus.Debugf("failed to extract cluster that the object [%s:%s] of kind [%s] belongs to", ns, name, kind)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I fear that this makes it into production, and a user sees the logs and assumes that something in rancher is failing, causing them to raise a ticket.

Suggested change
logrus.Debugf("failed to extract cluster that the object [%s:%s] of kind [%s] belongs to", ns, name, kind)
logrus.Debugf("unable to extract cluster that the object [%s:%s] of kind [%s] belongs to", ns, name, kind)

@@ -17,6 +17,8 @@ func ObjectInCluster(cluster string, obj interface{}) bool {
// Check if the object implements the interface, this is best case and
// what objects should strive to be
if o, ok := obj.(ObjectClusterName); ok {
clusterName := o.ObjClusterName()
debugLogsClusterExtraction(clusterName, obj)
return o.ObjClusterName() == cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return o.ObjClusterName() == cluster
return clusterName == cluster

@rmweir
Copy link
Contributor Author

rmweir commented Mar 24, 2023

@KevinJoiner We are trying to figure out how often this happens. If it happens frequently then that is a problem. I don't think spam will be much of an issue since it's debug. The handler also have a built in backoff so it should be throttled.

Copy link
Contributor

@cbron cbron left a comment

Choose a reason for hiding this comment

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

lgtm, since it is debug only. Sort of sucks it will run for every case, but should be minimal.

@rmweir rmweir merged commit ae12f16 into rancher:master Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants