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

[WIP] Added status metrics for gluster daemon and exporter #107

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

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 <arun.iiird@gmail.com>

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?

@@ -15,6 +15,22 @@ var (
peerIDPattern = regexp.MustCompile("[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}")
)

// 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