minio / mint

Collection of tests to detect overall correctness of MinIO server.
Apache License 2.0
82 stars 50 forks source link

Update mc to output new mint log format #114

Closed nitisht closed 7 years ago

nitisht commented 7 years ago

mc logs need to be updated in the format below so that mint logs can be easily parsed.

{
    "name":"mc", // SDK Name
    "function":"testPresignGetObject", // Test function name
    "description": "test function description (optional)", //  Test function description
    "args":"", // key value map, varName:value. Only arguements that devs may be interested in
    "duration":"", // duration of the whole test
    "status":"", // can be PASS, FAIL, NA
    "alert":"failed to download pre-signed object(optional)", // error related, human readable message. Should be taken care of if present
    "error":"stack-trace/exception message(only in case of failure)" // actual low level exception/error thrown by the program
}
ebozduman commented 7 years ago

Question: Do we need to make distinction between required/mandatory and optional arguments for the test case when the arguments are listed in the logs? aws-sdk-ruby has that information and if decided, it can be included in the "args": field.

Currently, aws-sdk-ruby provides an array of strings for "args": field, like:

{
  "name": "aws-sdk-ruby",
  "function": "copyObjectTest",
  "args": [
    "source_bucket_name",
    "target_bucket_name",
    "data_dir",
    "source_file_name",
    "target_file_name"
  ],
  "comment": "Tests copyObject api command",
  "message": "NULL",
  "error": "NULL",
  "status": "PASS"
}

Please advise.

ebozduman commented 7 years ago

I think we also need to discuss how to differentiate between the same test running with different arg values to test different scenarios, like invalid inputs etc.. At this point, "comment": being set inside the test method, there is no way to differentiate them from each other. First thing comes to my mind is to have caller define the purpose of the test in the comment, and have comment as an additional argument for the test, but I don't like it. I am sure there is a better way to handle this situation. Any thoughts?

nitisht commented 7 years ago

@ebozduman I think the args field doesn't really solve a purpose. Instead we can have comment moved just below the function section and have comment indicate short description of what the test is supposed to validate.

kannappanr commented 7 years ago

@NitishT ,

There was a change in the log format. You are missing "message" field. AB asked us to remove the description field and add the parameters in the function field. Here is a modified version of a sample log entry

{ "name":"mc", // SDK Name "function":"testPresignGetObject(int index)", // Test function name "args":"", // key value map, varName:value. Only arguements that devs may be interested in "duration":"1.5 s", // duration of the whole test in seconds "status":"", // can be PASS, FAIL, NA "alert":"Information like whether this is a Blocker/ Gateway, Server etc can go here", "message":" descriptive error message" // error related, human readable message. Should be taken care of if present "error":"stack-trace/exception message(only in case of failure)" // actual low level exception/error thrown by the program }

ebozduman commented 7 years ago

I think there are couple of things I'd like to suggest/clarify and confirm with the team and AB:

  1. "function": My understanding is that this field was going to be the test method name and its arguments, like bucketExistsNegativeTest(bucket_name), not the api name bucketExists(bucket_name).
  2. "description": I'd like to suggest keeping this optional field for those test cases which may need some extra words to explain what is happening inside the test method when the name of the test is not self-explanatory or sufficient enough.
balamurugana commented 7 years ago
"function": My understanding is that this field was going to be the test method name and its arguments, like bucketExistsNegativeTest(bucket_name), not the api name bucketExists(bucket_name).

For SDK, my recommendation is to use test method name than API name. For tools, full command line like mc cp file minio/mybucket/myobject

"description": I'd like to suggest keeping this optional field for those test cases which may need some extra words to explain what is happening inside the test method when the name of the test is not self-explanatory.

As we send only fields with values, I am OK with this

nitisht commented 7 years ago

@kannappanr @ebozduman @balamurugana can we have the final/consolidated log format pasted here, so it doesn't lead to rework.

kannappanr commented 7 years ago

Latest Log Format.

{
"name":"mc", // SDK Name
"function":"makeBucket(String bucketName)", // API name
"args":"", // key value map, varName:value. Only arguements that devs may be interested in
"duration":"15 ", // duration of the whole test in milli seconds
"status":"", // can be PASS, FAIL, NA
"alert":"Information like whether this is a Blocker/ Gateway, Server etc can go here",
"message":" descriptive error message" // error related, human readable message. Should be taken care of if present
"error":"stack-trace/exception message(only in case of failure)" // actual low level exception/error thrown by the program
}
ebozduman commented 7 years ago

The above information Kannappan put together came from an informal meeting Kannappan, AB and I had. We can definitely have another meeting to talk about the logic and reasons why the latest format is decided.

nitisht commented 7 years ago

The above information Kannappan put together came from an informal meeting Kannappan, AB and I had. We can definitely have another meeting to talk about the logic and reasons why the latest format is decided.

Thanks @ebozduman . We can do that, but as long as this format is agreed upon and approved by @abperiasamy lets adhere to this. We'll review current PRs and implement new logging based on this format.