V-style logging with glog and check errs #18

cxuu · 2018-11-14T23:27:57Z

This PR enables level logging so http requests and responses (often verbose) are only logged when the user asks with -v flag. More information about glog can be found https://godoc.org/github.com/golang/glog.

Originally, go-runscope uses native go log, which is hard to filter and interleaves with the application logs. There is hacky workaround such as log.SetOutput(ioutil.Discard) but that does not play well with clients also using go native log.

This PR also add the checks for errors by ReadAll() and json.Unmarshal() in client.go. The recent commits also fix the Makefile with shell keywords in front of shell commands, linter issues in bucket.go, and a typo in client_test.go.

cxuu · 2018-11-14T23:29:37Z

@ewilde PTAL

cxuu · 2018-11-15T00:03:43Z

It seems the CI is having trouble installing the linter. On my local, make build is passing with output reproduced below.

go get github.com/golang/lint/golint
go test  -v=1 --logtostderr
=== RUN   TestCreateBucket
--- PASS: TestCreateBucket (3.42s)
=== RUN   TestDeleteBuckets
--- PASS: TestDeleteBuckets (7.89s)
=== RUN   TestListBuckets
--- PASS: TestListBuckets (6.75s)
=== RUN   TestReadBucket
--- PASS: TestReadBucket (3.85s)
=== RUN   TestBucketReadFromResponse
--- PASS: TestBucketReadFromResponse (0.00s)
=== RUN   TestDeserializeResult
--- PASS: TestDeserializeResult (0.00s)
=== RUN   TestCreateSharedEnvironment
--- PASS: TestCreateSharedEnvironment (4.76s)
=== RUN   TestCreateTestEnvironment
--- PASS: TestCreateTestEnvironment (6.53s)
=== RUN   TestReadEnvironmentFromResponse
--- PASS: TestReadEnvironmentFromResponse (0.00s)
=== RUN   TestListSharedEnvironment
--- PASS: TestListSharedEnvironment (6.97s)
=== RUN   TestCreateSchedule
--- PASS: TestCreateSchedule (7.17s)
=== RUN   TestReadSchedule
--- PASS: TestReadSchedule (7.89s)
=== RUN   TestUpdateSchedule
--- PASS: TestUpdateSchedule (9.73s)
=== RUN   TestDeleteSchedule
--- PASS: TestDeleteSchedule (7.89s)
=== RUN   TestListSchedules
--- PASS: TestListSchedules (7.32s)
=== RUN   TestListIntegration
--- PASS: TestListIntegration (0.57s)
=== RUN   TestCreateTestStep
--- PASS: TestCreateTestStep (6.25s)
=== RUN   TestReadTestStep
--- PASS: TestReadTestStep (6.76s)
=== RUN   TestUpdateTestStep
--- PASS: TestUpdateTestStep (7.15s)
=== RUN   TestDeleteTestStep
--- PASS: TestDeleteTestStep (6.67s)
=== RUN   TestValidationRequestTypeMissingMethod
--- PASS: TestValidationRequestTypeMissingMethod (0.00s)
=== RUN   TestValidationRequestTypeGetIncludesBody
--- PASS: TestValidationRequestTypeGetIncludesBody (0.00s)
=== RUN   TestCreateTest
--- PASS: TestCreateTest (4.65s)
=== RUN   TestReadTest
--- PASS: TestReadTest (5.39s)
=== RUN   TestUpdateTest
--- PASS: TestUpdateTest (5.73s)
=== RUN   TestUpdateTestUsingPartiallyFilledOutObject
--- PASS: TestUpdateTestUsingPartiallyFilledOutObject (5.53s)
=== RUN   TestReadFromResponse
--- PASS: TestReadFromResponse (0.00s)
=== RUN   TestListsTests
--- PASS: TestListsTests (6.86s)
=== RUN   TestListsTestsMoreThan10
--- PASS: TestListsTestsMoreThan10 (28.64s)
PASS
2018/11/14 15:56:50 deleting bucket name: test key: m6iovwj1pk1q 
ok  	github.com/ewilde/go-runscope	167.856s
go vet github.com/ewilde/go-runscope github.com/ewilde/go-runscope/examples
golint github.com/ewilde/go-runscope github.com/ewilde/go-runscope/examples

canthefason · 2018-11-16T21:04:31Z

if switching to glog is going to be a thing, to discern the action items from the request/response bodies, it would be nice to level them separately.

glog.V(1).Infof("[DEBUG] creating bucket %s", bucket.Name)
glog.V(2).Infof("[DEBUG] 	request: POST %s %#v", "/buckets", data)

ewilde · 2018-11-16T21:21:09Z

@charleschengxu thank you for the PR, i'm not familiar with glog please allow me some time to evaluate. We use go-runscope in https://github.com/terraform-providers/terraform-provider-runscope, so want to check if adopting this library would impact that.

@canthefason your comment seems like a good suggestions, @charleschengxu thoughts?

GitHub
terraform-providers/terraform-provider-runscope
Terraform runscope provider. Contribute to terraform-providers/terraform-provider-runscope development by creating an account on GitHub.

ewilde · 2018-11-16T21:23:00Z

client.go

 		return response, fmt.Errorf("Status: %s Error reading %s: %s, reason: %q",
 			resp.Status, resourceType, resourceName, errorResp.ErrorMessage)
 	}

-	json.Unmarshal(bodyBytes, &response)
+	if err = json.Unmarshal(bodyBytes, &response); err != nil {


ewilde · 2018-11-16T21:23:18Z

client_test.go

@@ -52,7 +52,7 @@ func clientConfigure() *Client {
 func testPreCheck(t *testing.T) {
 	skip := os.Getenv("RUNSCOPE_ACC") == ""
 	if skip {
-		t.Log("runscope client.go tests require setting RUNSCOPE")
+		t.Log("runscope client.go tests require setting RUNSCOPE_ACC")


canthefason · 2018-11-16T22:02:25Z

bucket.go

 type ListTestsInput struct {
 	BucketName string
 	Count      int
 }

-// ListBuckets lists all buckets for an account
+// ListTests lists all buckets for an account


~~all buckets for an account~~/all tests for a bucket

cxuu · 2018-11-16T23:29:28Z

Yes @canthefason has a good point and I added the separate leveling in the latest commit.

@ewilde The logging utility change is unlikely to impact existing clients. If command line inputs are not accessible, one could resort to something like flag.Lookup("v").Value.Set("2") and flag.Lookup("logtostderr").Value.Set("true") in the application.

ewilde · 2018-11-26T08:45:03Z

@charleschengxu @canthefason I've had sometime this morning to look at this. I do have some reservations introducing a logging library dependency to a package. I would like the end-user of the library to be able to pick their own favourite. The list is pretty big! https://awesome-go.com/#logging

To that end, I had a go at adding some configuration to allow the end-user to drop in their logging library of choice #19 . What do you think?

For you to configure glog, it would probably look something like

handler := func(level int, format string, args ...interface{}) {
    glog.V(level).Infof("Any custom prefix or not %s", fmt.Sprintf(format, args...)) 
}

RegisterLogHandlers(handler, handler, handler)

Awesome Go
A curated list of awesome #Golang frameworks, libraries and software

cxuu · 2018-11-27T19:03:20Z

@ewilde Thanks for the other PR. Once it is merged, I will rebase from it and register the logging utility.

cxuu · 2018-11-28T19:30:21Z

@ewilde I have removed glog from this PR and kept other patches. PTAL

ewilde

Thank you

cxuu force-pushed the patches branch from 307ad58 to 9ea82c8 Compare November 14, 2018 23:36

ewilde added enhancement evaluating labels Nov 16, 2018

ewilde reviewed Nov 16, 2018

View reviewed changes

canthefason reviewed Nov 16, 2018

View reviewed changes

Charles Xu added 3 commits November 28, 2018 11:18

Level logging with glog and check errs

5063d98

fix typo and linter and makefile

58d51d6

address code review

76687d8

cxuu force-pushed the patches branch 2 times, most recently from 4e14e9a to 7935589 Compare November 28, 2018 19:24

remove glog vendor dep

f7dd7e2

cxuu force-pushed the patches branch from 7935589 to f7dd7e2 Compare November 28, 2018 19:25

ewilde approved these changes Nov 28, 2018

View reviewed changes

ewilde merged commit 80a0796 into ewilde:master Nov 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V-style logging with glog and check errs #18

V-style logging with glog and check errs #18

cxuu commented Nov 14, 2018 •

edited

Loading

cxuu commented Nov 14, 2018

cxuu commented Nov 15, 2018

canthefason commented Nov 16, 2018

ewilde commented Nov 16, 2018 •

edited by unfurl-links bot

Loading

ewilde Nov 16, 2018

ewilde Nov 16, 2018

canthefason Nov 16, 2018

cxuu Nov 16, 2018

cxuu commented Nov 16, 2018

ewilde commented Nov 26, 2018 •

edited

Loading

cxuu commented Nov 27, 2018

cxuu commented Nov 28, 2018

ewilde left a comment

V-style logging with glog and check errs #18

V-style logging with glog and check errs #18

Conversation

cxuu commented Nov 14, 2018 • edited Loading

cxuu commented Nov 14, 2018

cxuu commented Nov 15, 2018

canthefason commented Nov 16, 2018

ewilde commented Nov 16, 2018 • edited by unfurl-links bot Loading

ewilde Nov 16, 2018

Choose a reason for hiding this comment

ewilde Nov 16, 2018

Choose a reason for hiding this comment

canthefason Nov 16, 2018

Choose a reason for hiding this comment

cxuu Nov 16, 2018

Choose a reason for hiding this comment

cxuu commented Nov 16, 2018

ewilde commented Nov 26, 2018 • edited Loading

cxuu commented Nov 27, 2018

cxuu commented Nov 28, 2018

ewilde left a comment

Choose a reason for hiding this comment

cxuu commented Nov 14, 2018 •

edited

Loading

ewilde commented Nov 16, 2018 •

edited by unfurl-links bot

Loading

ewilde commented Nov 26, 2018 •

edited

Loading