Skip to content

Commit

Permalink
logging: Log only for unhandled errors, remove all the debug logging. (
Browse files Browse the repository at this point in the history
…minio#1652)

This patch brings in the removal of debug logging altogether, instead
we bring in the functionality of being able to trace the errors properly
pointing back to the origination of the problem.

To enable tracing you need to enable "MINIO_TRACE" set to "1" or "true"
environment variable which would print back traces whenever there is an
error which is unhandled or at the handler layer.

By default this tracing is turned off and only user level logging is
provided.
  • Loading branch information
harshavardhana authored and abperiasamy committed May 16, 2016
1 parent 8828fd1 commit 9472299
Show file tree
Hide file tree
Showing 38 changed files with 166 additions and 731 deletions.
2 changes: 1 addition & 1 deletion access-key.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ var isValidAccessKey = regexp.MustCompile(`^[a-zA-Z0-9\\-\\.\\_\\~]{5,20}$`)
// mustGenAccessKeys - must generate access credentials.
func mustGenAccessKeys() (creds credential) {
creds, err := genAccessKeys()
fatalIf(err, "Unable to generate access keys.", nil)
fatalIf(err, "Unable to generate access keys.")
return creds
}

Expand Down
6 changes: 3 additions & 3 deletions accesslog-handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ type LogMessage struct {

func (h *accessLogHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
message, err := getLogMessage(w, req)
fatalIf(err, "Unable to extract http message.", nil)
fatalIf(err, "Unable to parse HTTP request and response fields.")
_, err = h.accessLogFile.Write(message)
fatalIf(err, "Writing to log file failed.", nil)
fatalIf(err, "Unable to log HTTP access.")

h.Handler.ServeHTTP(w, req)
}
Expand Down Expand Up @@ -112,7 +112,7 @@ func getLogMessage(w http.ResponseWriter, req *http.Request) ([]byte, error) {
// setAccessLogHandler logs requests
func setAccessLogHandler(h http.Handler) http.Handler {
file, err := os.OpenFile("access.log", os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
fatalIf(err, "Unable to open access log.", nil)
fatalIf(err, "Failed top open access log.")

return &accessLogHandler{Handler: h, accessLogFile: file}
}
2 changes: 0 additions & 2 deletions auth-handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,10 @@ func sumMD5(data []byte) []byte {
// Verify if request has valid AWS Signature Version '4'.
func isReqAuthenticated(r *http.Request) (s3Error APIErrorCode) {
if r == nil {
errorIf(errInvalidArgument, "HTTP request cannot be empty.", nil)
return ErrInternalError
}
payload, err := ioutil.ReadAll(r.Body)
if err != nil {
errorIf(err, "Unable to read HTTP body.", nil)
return ErrInternalError
}
// Verify Content-Md5, if payload is set.
Expand Down
30 changes: 15 additions & 15 deletions bucket-handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func enforceBucketPolicy(action string, bucket string, reqURL *url.URL) (s3Error
// Read saved bucket policy.
policy, err := readBucketPolicy(bucket)
if err != nil {
errorIf(err, "GetBucketPolicy failed.", nil)
errorIf(err, "Unable read bucket policy.")
switch err.(type) {
case BucketNotFound:
return ErrNoSuchBucket
Expand All @@ -50,7 +50,7 @@ func enforceBucketPolicy(action string, bucket string, reqURL *url.URL) (s3Error
// Parse the saved policy.
bucketPolicy, err := parseBucketPolicy(policy)
if err != nil {
errorIf(err, "Parse policy failed.", nil)
errorIf(err, "Unable to parse bucket policy.")
return ErrAccessDenied
}

Expand Down Expand Up @@ -117,7 +117,7 @@ func (api objectAPIHandlers) GetBucketLocationHandler(w http.ResponseWriter, r *
}

if _, err := api.ObjectAPI.GetBucketInfo(bucket); err != nil {
errorIf(err, "GetBucketInfo failed.", nil)
errorIf(err, "Unable to fetch bucket info.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
return
}
Expand Down Expand Up @@ -180,7 +180,7 @@ func (api objectAPIHandlers) ListMultipartUploadsHandler(w http.ResponseWriter,

listMultipartsInfo, err := api.ObjectAPI.ListMultipartUploads(bucket, prefix, keyMarker, uploadIDMarker, delimiter, maxUploads)
if err != nil {
errorIf(err, "ListMultipartUploads failed.", nil)
errorIf(err, "Unable to list multipart uploads.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
return
}
Expand Down Expand Up @@ -252,7 +252,7 @@ func (api objectAPIHandlers) ListObjectsHandler(w http.ResponseWriter, r *http.R
writeSuccessResponse(w, encodedSuccessResponse)
return
}
errorIf(err, "ListObjects failed.", nil)
errorIf(err, "Unable to list objects.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
}

Expand Down Expand Up @@ -306,7 +306,7 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R
writeSuccessResponse(w, encodedSuccessResponse)
return
}
errorIf(err, "ListBuckets failed.", nil)
errorIf(err, "Unable to list buckets.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
}

Expand Down Expand Up @@ -352,15 +352,15 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,

// Read incoming body XML bytes.
if _, err := io.ReadFull(r.Body, deleteXMLBytes); err != nil {
errorIf(err, "DeleteMultipleObjects failed.", nil)
errorIf(err, "Unable to read HTTP body.")
writeErrorResponse(w, r, ErrInternalError, r.URL.Path)
return
}

// Unmarshal list of keys to be deleted.
deleteObjects := &DeleteObjectsRequest{}
if err := xml.Unmarshal(deleteXMLBytes, deleteObjects); err != nil {
errorIf(err, "DeleteMultipartObjects xml decoding failed.", nil)
errorIf(err, "Unable to unmarshal delete objects request XML.")
writeErrorResponse(w, r, ErrMalformedXML, r.URL.Path)
return
}
Expand All @@ -375,7 +375,7 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
ObjectName: object.ObjectName,
})
} else {
errorIf(err, "DeleteObject failed.", nil)
errorIf(err, "Unable to delete object.")
deleteErrors = append(deleteErrors, DeleteError{
Code: errorCodeResponse[toAPIErrorCode(err)].Code,
Message: errorCodeResponse[toAPIErrorCode(err)].Description,
Expand Down Expand Up @@ -423,7 +423,7 @@ func (api objectAPIHandlers) PutBucketHandler(w http.ResponseWriter, r *http.Req
// Make bucket.
err := api.ObjectAPI.MakeBucket(bucket)
if err != nil {
errorIf(err, "MakeBucket failed.", nil)
errorIf(err, "Unable to create a bucket.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
return
}
Expand Down Expand Up @@ -467,14 +467,14 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
// be loaded in memory, the remaining being put in temporary files.
reader, err := r.MultipartReader()
if err != nil {
errorIf(err, "Unable to initialize multipart reader.", nil)
errorIf(err, "Unable to initialize multipart reader.")
writeErrorResponse(w, r, ErrMalformedPOSTRequest, r.URL.Path)
return
}

fileBody, formValues, err := extractHTTPFormValues(reader)
if err != nil {
errorIf(err, "Unable to parse form values.", nil)
errorIf(err, "Unable to parse form values.")
writeErrorResponse(w, r, ErrMalformedPOSTRequest, r.URL.Path)
return
}
Expand All @@ -494,7 +494,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
}
md5Sum, err := api.ObjectAPI.PutObject(bucket, object, -1, fileBody, nil)
if err != nil {
errorIf(err, "PutObject failed.", nil)
errorIf(err, "Unable to create object.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
return
}
Expand Down Expand Up @@ -540,7 +540,7 @@ func (api objectAPIHandlers) HeadBucketHandler(w http.ResponseWriter, r *http.Re
}

if _, err := api.ObjectAPI.GetBucketInfo(bucket); err != nil {
errorIf(err, "GetBucketInfo failed.", nil)
errorIf(err, "Unable to fetch bucket info.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
return
}
Expand All @@ -565,7 +565,7 @@ func (api objectAPIHandlers) DeleteBucketHandler(w http.ResponseWriter, r *http.
}

if err := api.ObjectAPI.DeleteBucket(bucket); err != nil {
errorIf(err, "DeleteBucket failed.", nil)
errorIf(err, "Unable to delete a bucket.")
writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
return
}
Expand Down
12 changes: 6 additions & 6 deletions bucket-policy-handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func bucketPolicyActionMatch(action string, statement policyStatement) bool {
for _, policyAction := range statement.Actions {
// Policy action can be a regex, validate the action with matching string.
matched, err := regexp.MatchString(policyAction, action)
fatalIf(err, "Invalid pattern, please verify the pattern string.", nil)
fatalIf(err, "Invalid action \"%s\" in bucket policy.", action)
if matched {
return true
}
Expand Down Expand Up @@ -190,15 +190,15 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht
// bucket policies are limited to 20KB in size, using a limit reader.
bucketPolicyBuf, err := ioutil.ReadAll(io.LimitReader(r.Body, maxAccessPolicySize))
if err != nil {
errorIf(err, "Reading policy failed.", nil)
errorIf(err, "Unable to read bucket policy.")
writeErrorResponse(w, r, ErrInternalError, r.URL.Path)
return
}

// Parse bucket policy.
bucketPolicy, err := parseBucketPolicy(bucketPolicyBuf)
if err != nil {
errorIf(err, "Unable to parse bucket policy.", nil)
errorIf(err, "Unable to parse bucket policy.")
writeErrorResponse(w, r, ErrInvalidPolicyDocument, r.URL.Path)
return
}
Expand All @@ -211,7 +211,7 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht

// Save bucket policy.
if err := writeBucketPolicy(bucket, bucketPolicyBuf); err != nil {
errorIf(err, "SaveBucketPolicy failed.", nil)
errorIf(err, "Unable to write bucket policy.")
switch err.(type) {
case BucketNameInvalid:
writeErrorResponse(w, r, ErrInvalidBucketName, r.URL.Path)
Expand Down Expand Up @@ -245,7 +245,7 @@ func (api objectAPIHandlers) DeleteBucketPolicyHandler(w http.ResponseWriter, r

// Delete bucket access policy.
if err := removeBucketPolicy(bucket); err != nil {
errorIf(err, "DeleteBucketPolicy failed.", nil)
errorIf(err, "Unable to remove bucket policy.")
switch err.(type) {
case BucketNameInvalid:
writeErrorResponse(w, r, ErrInvalidBucketName, r.URL.Path)
Expand Down Expand Up @@ -282,7 +282,7 @@ func (api objectAPIHandlers) GetBucketPolicyHandler(w http.ResponseWriter, r *ht
// Read bucket access policy.
p, err := readBucketPolicy(bucket)
if err != nil {
errorIf(err, "GetBucketPolicy failed.", nil)
errorIf(err, "Unable to read bucket policy.")
switch err.(type) {
case BucketNameInvalid:
writeErrorResponse(w, r, ErrInvalidBucketName, r.URL.Path)
Expand Down
2 changes: 1 addition & 1 deletion certs.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func getCertsPath() (string, error) {
// mustGetCertsPath must get certs path.
func mustGetCertsPath() string {
certsPath, err := getCertsPath()
fatalIf(err, "Unable to retrieve certs path.", nil)
fatalIf(err, "Failed to get certificate path.")
return certsPath
}

Expand Down
4 changes: 4 additions & 0 deletions commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ var globalFlags = []cli.Flag{
Value: mustGetConfigPath(),
Usage: "Path to configuration folder.",
},
cli.BoolFlag{
Name: "quiet",
Usage: "Suppress chatty output.",
},
}

// registerCommand registers a cli command.
Expand Down
24 changes: 12 additions & 12 deletions config-migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,18 @@ func purgeV1() {
if err != nil && os.IsNotExist(err) {
return
}
fatalIf(err, "Unable to load config version ‘1’.", nil)
fatalIf(err, "Unable to load config version ‘1’.")

if cv1.Version == "1" {
console.Println("Unsupported config version ‘1’ found, removed successfully.")
console.Println("Removed unsupported config version ‘1’.")
/// Purge old fsUsers.json file
configPath, err := getConfigPath()
fatalIf(err, "Unable to retrieve config path.", nil)
fatalIf(err, "Unable to retrieve config path.")

configFile := filepath.Join(configPath, "fsUsers.json")
os.RemoveAll(configFile)
}
fatalIf(errors.New(""), "Unexpected version found ‘"+cv1.Version+"’, cannot migrate.", nil)
fatalIf(errors.New(""), "Failed to migrate unrecognized config version ‘"+cv1.Version+"’.")
}

// Version '2' to '3' config migration adds new fields and re-orders
Expand All @@ -61,7 +61,7 @@ func migrateV2ToV3() {
if err != nil && os.IsNotExist(err) {
return
}
fatalIf(err, "Unable to load config version ‘2’.", nil)
fatalIf(err, "Unable to load config version ‘2’.")
if cv2.Version != "2" {
return
}
Expand Down Expand Up @@ -98,14 +98,14 @@ func migrateV2ToV3() {
srvConfig.Logger.Syslog = slogger

qc, err := quick.New(srvConfig)
fatalIf(err, "Unable to initialize config.", nil)
fatalIf(err, "Unable to initialize config.")

configFile, err := getConfigFile()
fatalIf(err, "Unable to get config file.", nil)
fatalIf(err, "Unable to get config file.")

// Migrate the config.
err = qc.Save(configFile)
fatalIf(err, "Migrating from version ‘"+cv2.Version+"’ to ‘"+srvConfig.Version+"’ failed.", nil)
fatalIf(err, "Failed to migrate config from ‘"+cv2.Version+"’ to ‘"+srvConfig.Version+"’ failed.")

console.Println("Migration from version ‘" + cv2.Version + "’ to ‘" + srvConfig.Version + "’ completed successfully.")
}
Expand All @@ -118,7 +118,7 @@ func migrateV3ToV4() {
if err != nil && os.IsNotExist(err) {
return
}
fatalIf(err, "Unable to load config version ‘3’.", nil)
fatalIf(err, "Unable to load config version ‘3’.")
if cv3.Version != "3" {
return
}
Expand All @@ -137,12 +137,12 @@ func migrateV3ToV4() {
srvConfig.Logger.Syslog = cv3.Logger.Syslog

qc, err := quick.New(srvConfig)
fatalIf(err, "Unable to initialize the quick config.", nil)
fatalIf(err, "Unable to initialize the quick config.")
configFile, err := getConfigFile()
fatalIf(err, "Unable to get config file.", nil)
fatalIf(err, "Unable to get config file.")

err = qc.Save(configFile)
fatalIf(err, "Migrating from version ‘"+cv3.Version+"’ to ‘"+srvConfig.Version+"’ failed.", nil)
fatalIf(err, "Failed to migrate config from ‘"+cv3.Version+"’ to ‘"+srvConfig.Version+"’ failed.")

console.Println("Migration from version ‘" + cv3.Version + "’ to ‘" + srvConfig.Version + "’ completed successfully.")
}
4 changes: 2 additions & 2 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func getConfigPath() (string, error) {
// mustGetConfigPath must get server config path.
func mustGetConfigPath() string {
configPath, err := getConfigPath()
fatalIf(err, "Unable to get config path.", nil)
fatalIf(err, "Unable to get config path.")
return configPath
}

Expand All @@ -73,7 +73,7 @@ func isConfigFileExists() bool {
// mustGetConfigFile must get server config file.
func mustGetConfigFile() string {
configFile, err := getConfigFile()
fatalIf(err, "Unable to get config file.", nil)
fatalIf(err, "Unable to get config file.")

return configFile
}
Expand Down
12 changes: 5 additions & 7 deletions docs/logging.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
### Logging.

- `log.Fatalf`
- `log.Errorf`
- `log.Warnf`
- `log.Infof`
- `fatalIf` - wrapper function which takes error and prints jsonic error messages.
- `errorIf` - similar to fatalIf but doesn't exit on err != nil.

Logging is enabled across the codebase. There are three types of logging supported.
Supported logging targets.

- console
- file
Expand All @@ -20,11 +18,11 @@ Sample logger section from `~/.minio/config.json`
"file": {
"enable": false,
"fileName": "",
"level": "trace"
"level": "error"
},
"syslog": {
"enable": false,
"address": "",
"level": "info"
"level": "error"
}
```
Loading

0 comments on commit 9472299

Please sign in to comment.