Skip to content

Conversation

@aruniiird
Copy link
Collaborator

Fixes: /issues/85

@ghost ghost assigned aruniiird Dec 24, 2018
@ghost ghost added the in progress label Dec 24, 2018
@aruniiird
Copy link
Collaborator Author

@aravindavk @shtripat @sidharthanup
pending status work for gluster_operator, gluster_block

Signed-off-by: Arun Kumar Mohan <[email protected]>

const (
// GDaemonLabel provides static label to info/error provided from this metrics
GDaemonLabel = "Gluster_Daemon_Status"
Copy link

Choose a reason for hiding this comment

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

What is the purpose served by this error message label? We are not using such labeling for other error messages. Do we really need this as all the gl-exporter logs go to a separate log file? @aravindavk suggestions?

}
} else if conf.GlusterMgmt == glusterutils.MgmtGlusterd2 {
if conf.Glusterd2Endpoint == "" {
return errors.New("[" + GDaemonLabel + "] Empty GD2 Endpoint")
Copy link

Choose a reason for hiding this comment

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

I feel better to use fmt.Sprintf() for formatting strings.

if err != nil {
return errors.New("[" + GDaemonLabel + "] Error: " + err.Error())
}
log.Println("GD Management: ", conf.GlusterMgmt)
Copy link

Choose a reason for hiding this comment

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

Remove this. I feel this was added for debugging purpose.

}
log.Println("GD Management: ", conf.GlusterMgmt)
genrlLbls := prometheus.Labels{
"name": "Glusterd_Status",
Copy link

Choose a reason for hiding this comment

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

I feel we follow all small case separated by underscore so use the same way for uniformity.

if err != nil {
log.WithError(err).WithFields(log.Fields{
"peer": peerID,
"name": "Glusterd_Status",
Copy link

Choose a reason for hiding this comment

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

Same as above

"peer": peerID,
"name": "Glusterd_Status",
}).Errorln("["+GDaemonLabel+"] Error:", err)
glusterDaemonStatus.With(genrlLbls).Set(float64(0))
Copy link

Choose a reason for hiding this comment

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

Declare constants for states 0 and 1.

glusterDaemonStatus.With(genrlLbls).Set(float64(1))
}
genrlLbls = prometheus.Labels{
"name": "Gluster_Exporter_Status",
Copy link

Choose a reason for hiding this comment

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

all small cases separated by underscores?


// GDConfigFromInterface checks the given interface is compatible with 'GDConfigInterface'
// and returns a pointer to glusterutils.Config
func GDConfigFromInterface(iFace interface{}) (*Config, error) {
Copy link

Choose a reason for hiding this comment

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

Can you explain the purpose of this change and ideally this should go as part of another infra level PR and not as part of some feature PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants