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

Add logrus hook for sending SkaffoldLogEvents #6250

Merged

Conversation

MarlonGamez
Copy link
Contributor

Related: #5368

Description
This PR adds an implementation of a logrus.Hook type that will fire off SkaffoldLogEvents whenever we make calls to various logrus.Debug(), logrus.Info(), etc. functions

Follow-up work
Do some tracking of execution state in handler so that we can do some better setting of TaskId and SubtaskId in these events.

@MarlonGamez MarlonGamez requested a review from a team as a code owner July 19, 2021 23:44
@MarlonGamez MarlonGamez requested a review from briandealwis July 19, 2021 23:44
@google-cla google-cla bot added the cla: yes label Jul 19, 2021
@codecov
Copy link

codecov bot commented Jul 19, 2021

Codecov Report

Merging #6250 (bb2d7c6) into master (7453095) will increase coverage by 0.02%.
The diff coverage is 82.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6250      +/-   ##
==========================================
+ Coverage   70.84%   70.86%   +0.02%     
==========================================
  Files         490      490              
  Lines       22123    22162      +39     
==========================================
+ Hits        15673    15706      +33     
- Misses       5434     5440       +6     
  Partials     1016     1016              
Impacted Files Coverage Δ
pkg/skaffold/event/v2/logger.go 88.23% <82.35%> (-11.77%) ⬇️
cmd/skaffold/app/cmd/cmd.go 73.65% <100.00%> (+0.14%) ⬆️
pkg/skaffold/server/server.go 47.61% <0.00%> (ø)
...g/skaffold/kubernetes/portforward/entry_manager.go 91.78% <0.00%> (+0.11%) ⬆️
pkg/skaffold/runner/build.go 74.28% <0.00%> (+0.24%) ⬆️
pkg/skaffold/build/cache/retrieve.go 64.40% <0.00%> (+0.30%) ⬆️
pkg/skaffold/kubernetes/status/status_check.go 52.40% <0.00%> (+0.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7453095...bb2d7c6. Read the comment docs.

@@ -40,7 +43,7 @@ func (l logger) Write(p []byte) (int, error) {
handler.handleSkaffoldLogEvent(&proto.SkaffoldLogEvent{
TaskId: fmt.Sprintf("%s-%d", l.Phase, handler.iteration),
SubtaskId: l.SubtaskID,
Level: 0,
Level: -1,
Copy link
Contributor

Choose a reason for hiding this comment

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

what does the level -1 represent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is basically just to avoid marking normal output with a level, since they technically don't have one as they don't come from any logging library. I wasn't sure how to best represent this. I can put it in a variable to make things clear or leave a comment to clarify if you think that's best

Copy link
Contributor

Choose a reason for hiding this comment

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

just a comment should suffice

Copy link
Contributor

@gsquared94 gsquared94 left a comment

Choose a reason for hiding this comment

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

lgtm

@tejal29
Copy link
Contributor

tejal29 commented Jul 20, 2021

Nice!

{"result":{"timestamp":"2021-07-20T16:34:25.308073Z","skaffoldLogEvent":{"task_id":"","subtask_id":"","level":"DEBUG","message":"Executing template \u0026{envTemplate 0xc001001560 0xc0003d2a40  } with environment map[DISPLAY:/private/tmp/com.apple.launchd.gcpy9xhATO/org.xquartz:0 HOME:/Users/tejaldesai LANG:en_US.UTF-8 LESS:-R LOGNAME:tejaldesai LSCOLORS:Gxfxcxdxbxegedabagacad LaunchInstanceID:C7748237-58DA-4C3E-A5D5-0D8F00A941B5 OLDPWD:/Users/tejaldesai/workspace/skaffold PAGER:less PATH:/Users/tejaldesai/workspace/google-cloud-sdk/bin:/usr/local/opt/[email protected]/bin:/Users/tejaldesai/mdproxy/bin:/Users/tejaldesai/workspace/google-cloud-sdk/bin:/usr/local/opt/[email protected]/bin:/Users/tejaldesai/mdproxy/bin:/usr/local/git/current/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/local/go/bin:/opt/X11/bin:/Library/Apple/usr/bin PWD:/Users/tejaldesai/workspace/skaffold/examples/microservices SECURITYSESSIONID:186ac SHELL:/bin/zsh SHLVL:1 SK_SIGNING_PLUGIN:gnubbyagent SSH_AUTH_SOCK:/private/tmp/com.apple.launchd.8kIonfpvTD/Listeners TERM:xterm-256color TERM_PROGRAM:Apple_Terminal TERM_PROGRAM_VERSION:440 TERM_SESSION_ID:DCB2CB54-F728-4305-8E19-18D0364AC34A TMPDIR:/var/folders/gk/s778hvkj0lb0zl95ywvw0snr00cfc0/T/ USER:tejaldesai XPC_FLAGS:0x0 XPC_SERVICE_NAME:0 ZSH:/Users/tejaldesai/.oh-my-zsh _:/Users/tejaldesai/workspace/skaffold/examples/microservices/../../out/skaffold __CFBundleIdentifier:com.apple.Terminal]"}}}
{"result":{"timestamp":"2021-07-20T16:34:25.308420Z","skaffoldLogEvent":{"task_id":"","subtask_id":"","level":"DEBUG","message":"Found dependencies for dockerfile: [{app.go /go true}]"}}}

@MarlonGamez MarlonGamez merged commit ea2b8b5 into GoogleContainerTools:master Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants