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

Use current json.Poster in sdk, which uses stdout #624

Merged
merged 1 commit into from
Apr 18, 2019

Conversation

dpordomingo
Copy link
Contributor

@dpordomingo dpordomingo commented Apr 9, 2019

fix #601

json.Poster writes the comments in json format into the passed io.Writter
so if using os.Stdout, comments will be written in stdout, and logs in stderr

Then comments and logs can be separated:

$ lookout-sdk review --from HEAD^ --log-format=json 2> log.err
{"analyzer-name":"","file":"cmd/lookout-sdk/push.go","text":"The file has increased in 2 lines."}
{"analyzer-name":"","file":"cmd/lookout-sdk/push.go","line":27,"text":"This line exceeded 120 chars."}
{"analyzer-name":"","file":"cmd/lookout-sdk/review.go","text":"The file has increased in 2 lines."}
{"analyzer-name":"","file":"cmd/lookout-sdk/review.go","line":23,"text":"This line exceeded 120 chars."}
{"analyzer-name":"","file":"cmd/sdk-test/bblfsh_test.go","line":146,"text":"This line exceeded 120 chars."}
Click to see `log.err`, where `stderr` was redirected:
$ cat log.err
{"app":"lookout-sdk","level":"info","msg":"resolving to/from references","time":"2019-04-09T11:13:08.194352331+02:00"}
{"app":"lookout-sdk","level":"info","msg":"starting looking for Analyzer at ipv4://localhost:9930","time":"2019-04-09T11:13:08.196105525+02:00"}
{"app":"lookout-sdk","level":"info","msg":"starting a DataServer at ipv4://localhost:10301","time":"2019-04-09T11:13:08.196239204+02:00"}
{"addr":"localhost:9432","app":"lookout-sdk","level":"info","msg":"connection state changed to 'IDLE'","name":"bblfsh-proxy","time":"2019-04-09T11:13:08.196725494+02:00"}
{"addr":"localhost:9432","app":"lookout-sdk","level":"info","msg":"connection state changed to 'CONNECTING'","name":"bblfsh-proxy","time":"2019-04-09T11:13:08.196821332+02:00"}
{"app":"lookout-sdk","level":"info","msg":"processing pull request","provider":"","time":"2019-04-09T11:13:08.197346984+02:00"}
{"addr":"localhost:9432","app":"lookout-sdk","level":"info","msg":"connection state changed to 'READY'","name":"bblfsh-proxy","time":"2019-04-09T11:13:08.197541946+02:00"}
{"app":"lookout-sdk","config-file":"repository .lookout.yml","level":"warning","msg":"analyzer 'Dummy' required by configuration file isn't enabled on server","provider":"","time":"2019-04-09T11:13:08.200001107+02:00"}
{"app":"lookout-sdk","config-file":"repository .lookout.yml","level":"warning","msg":"analyzer 'gometalint-analyzer' required by configuration file isn't enabled on server","provider":"","time":"2019-04-09T11:13:08.200028102+02:00"}
{"app":"lookout-sdk","level":"info","msg":"New status","provider":"","status":3,"time":"2019-04-09T11:13:08.200052838+02:00"}
{"analyzer":"test-analyzer","app":"lookout-sdk","grpc.method":"NotifyReviewEvent","grpc.service":"pb.Analyzer","level":"info","msg":"gRPC unary client call started","provider":"","span.kind":"client","system":"grpc","time":"2019-04-09T11:13:08.200125721+02:00"}
{"analyzer":"test-analyzer","app":"lookout-sdk","grpc.method":"GetChanges","grpc.service":"pb.Data","level":"info","msg":"gRPC streaming server call started","provider":"","span.kind":"server","system":"grpc","time":"2019-04-09T11:13:14.772033431+02:00"}
{"analyzer":"test-analyzer","app":"lookout-sdk","duration":119246660,"grpc.code":0,"grpc.method":"GetChanges","grpc.service":"pb.Data","grpc.start_time":"2019-04-09T11:13:14+02:00","level":"info","msg":"gRPC streaming server call finished","provider":"","span.kind":"server","system":"grpc","time":"2019-04-09T11:13:14.891535334+02:00"}
{"analyzer":"test-analyzer","app":"lookout-sdk","duration":6692246859,"grpc.code":0,"grpc.method":"NotifyReviewEvent","grpc.service":"pb.Analyzer","grpc.start_time":"2019-04-09T11:13:08+02:00","level":"info","msg":"gRPC unary client call finished","provider":"","span.kind":"client","system":"grpc","time":"2019-04-09T11:13:14.892412722+02:00"}
{"app":"lookout-sdk","comments":5,"level":"info","msg":"posting analysis","provider":"","time":"2019-04-09T11:13:14.892496472+02:00"}
{"app":"lookout-sdk","level":"info","msg":"New status","provider":"","status":4,"time":"2019-04-09T11:13:14.892988574+02:00"}

And it also works if the logs are requested to be human readable:

$ lookout-sdk review --from HEAD^ 2> log.err
{"analyzer-name":"","file":"cmd/lookout-sdk/push.go","text":"The file has increased in 2 lines."}
{"analyzer-name":"","file":"cmd/lookout-sdk/push.go","line":27,"text":"This line exceeded 120 chars."}
{"analyzer-name":"","file":"cmd/lookout-sdk/review.go","text":"The file has increased in 2 lines."}
{"analyzer-name":"","file":"cmd/lookout-sdk/review.go","line":23,"text":"This line exceeded 120 chars."}
{"analyzer-name":"","file":"cmd/sdk-test/bblfsh_test.go","line":146,"text":"This line exceeded 120 chars."}
Click to see `log.err`, where `stderr` was redirected:
$ cat log.err
time="2019-04-09T11:20:29.943666171+02:00" level=info msg="resolving to/from references" app=lookout-sdk
time="2019-04-09T11:20:29.945240973+02:00" level=info msg="starting looking for Analyzer at ipv4://localhost:9930" app=lookout-sdk
time="2019-04-09T11:20:29.945317668+02:00" level=info msg="starting a DataServer at ipv4://localhost:10301" app=lookout-sdk
time="2019-04-09T11:20:29.945477562+02:00" level=info msg="connection state changed to 'IDLE'" addr="localhost:9432" app=lookout-sdk name=bblfsh-proxy
time="2019-04-09T11:20:29.94553613+02:00" level=info msg="connection state changed to 'CONNECTING'" addr="localhost:9432" app=lookout-sdk name=bblfsh-proxy
time="2019-04-09T11:20:29.945905718+02:00" level=info msg="connection state changed to 'READY'" addr="localhost:9432" app=lookout-sdk name=bblfsh-proxy
time="2019-04-09T11:20:29.945935143+02:00" level=info msg="processing pull request" app=lookout-sdk provider=
time="2019-04-09T11:20:29.946451612+02:00" level=warning msg="analyzer 'Dummy' required by configuration file isn't enabled on server" app=lookout-sdk config-file="repository .lookout.yml" provider=
time="2019-04-09T11:20:29.946471481+02:00" level=warning msg="analyzer 'gometalint-analyzer' required by configuration file isn't enabled on server" app=lookout-sdk config-file="repository .lookout.yml" provider=
time="2019-04-09T11:20:29.946493983+02:00" level=info msg="New status" app=lookout-sdk provider= status=pending
time="2019-04-09T11:20:29.946559007+02:00" level=info msg="gRPC unary client call started" analyzer=test-analyzer app=lookout-sdk grpc.method=NotifyReviewEvent grpc.service=pb.Analyzer provider= span.kind=client system=grpc
time="2019-04-09T11:20:30.712726763+02:00" level=info msg="gRPC streaming server call started" analyzer=test-analyzer app=lookout-sdk grpc.method=GetChanges grpc.service=pb.Data provider= span.kind=server system=grpc
time="2019-04-09T11:20:30.77662199+02:00" level=info msg="gRPC streaming server call finished" analyzer=test-analyzer app=lookout-sdk duration=63.882508ms grpc.code=OK grpc.method=GetChanges grpc.service=pb.Data grpc.start_time="2019-04-09T11:20:30+02:00" provider= span.kind=server system=grpc
time="2019-04-09T11:20:30.778021699+02:00" level=info msg="gRPC unary client call finished" analyzer=test-analyzer app=lookout-sdk duration=831.395964ms grpc.code=OK grpc.method=NotifyReviewEvent grpc.service=pb.Analyzer grpc.start_time="2019-04-09T11:20:29+02:00" provider= span.kind=client system=grpc
time="2019-04-09T11:20:30.778126638+02:00" level=info msg="posting analysis" app=lookout-sdk comments=5 provider=
time="2019-04-09T11:20:30.778329528+02:00" level=info msg="New status" app=lookout-sdk provider= status=success

json.Poster writes the comments in json format over the passed io.Writter
so if using os.Stdout comments will be written in stdout, and logs in stderr

Signed-off-by: David Pordomingo <[email protected]>
@dpordomingo dpordomingo added the enhancement New feature or request label Apr 9, 2019
@dpordomingo dpordomingo self-assigned this Apr 9, 2019
@dpordomingo dpordomingo requested a review from a team April 9, 2019 09:03
Copy link
Contributor

@smacker smacker left a comment

Choose a reason for hiding this comment

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

@dpordomingo do you mind to create issues for the improvements we discussed?

@smacker
Copy link
Contributor

smacker commented Apr 10, 2019

I would also prefer to add omit-empty to analyzer-name field in the struct.

@carlosms
Copy link
Contributor

Yeah, it would be nice if you could find out why analyzer-name is empty

@dpordomingo
Copy link
Contributor Author

@smacker yes, I did: #625

Will work on analyzer-name issue.

@carlosms
Copy link
Contributor

I took a look and the analyzer-name json field is empty because the lookout-sdk commands do not mock the contents of lookout.yml AnalyzerConfig, where the name is set. We only set the client.

I think we can merge this one and add an AnalyzerConfig later here https://github.com/src-d/lookout/blob/master/cmd/lookout-sdk/review.go#L61.

@carlosms carlosms merged commit eab90e9 into src-d:master Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass comments generated by lookout-tool review call to stdout
4 participants