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 debug log for work unit payloads #1146

Merged
merged 17 commits into from
Sep 27, 2024
Merged

Add debug log for work unit payloads #1146

merged 17 commits into from
Sep 27, 2024

Conversation

matoval
Copy link
Collaborator

@matoval matoval commented Sep 18, 2024

Add debug log for work unit payloads using RECEPTOR_PAYLOAD_DEBUG env var

Control output:
Screenshot from 2024-09-18 13-11-28

Control status:
Screenshot from 2024-09-18 13-04-05

Execution output:
image

Screenshot from 2024-09-18 13-04-33

Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 35.52632% with 49 lines in your changes missing coverage. Please review.

Project coverage is 43.74%. Comparing base (b9bab8f) to head (87d1972).
Report is 4 commits behind head on devel.

Files with missing lines Patch % Lines
pkg/workceptor/command.go 0.00% 22 Missing ⚠️
pkg/controlsvc/controlsvc.go 18.18% 17 Missing and 1 partial ⚠️
pkg/workceptor/stdio_utils.go 30.76% 8 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##            devel    #1146      +/-   ##
==========================================
+ Coverage   42.99%   43.74%   +0.74%     
==========================================
  Files          33       36       +3     
  Lines        6612     7265     +653     
==========================================
+ Hits         2843     3178     +335     
- Misses       3530     3845     +315     
- Partials      239      242       +3     
Files with missing lines Coverage Δ
pkg/logger/logger.go 45.39% <100.00%> (+25.09%) ⬆️
pkg/workceptor/stdio_utils.go 83.54% <30.76%> (-10.40%) ⬇️
pkg/controlsvc/controlsvc.go 73.44% <18.18%> (-4.46%) ⬇️
pkg/workceptor/command.go 45.93% <0.00%> (-3.69%) ⬇️

... and 7 files with indirect coverage changes

Components Coverage Δ
Go 43.26% <35.52%> (+0.26%) ⬆️
Receptorctl 49.31% <ø> (∅)

@AaronH88
Copy link
Contributor

AaronH88 commented Sep 19, 2024

Great work on this,
I am imagining using this now and I am throwing out ideas,

First idea, lets change RECEPTOR_PAYLOAD_DEBUG to RECEPTOR_PAYLOAD_TRACE_LEVEL which will expect an int
Now that we have different "levels" for tracing, lets build the debug message in a switch case where the higher the int number results in more of the payload data being printed.

Imagine level 1 we just print something like "Unix message" , "Network message".
Level 2 is like the full payload, which you have,
Then Level 3 could be the payload plus the work unit id. (which you got!)

Let me know what you think.

The next thing to think about is code reusability (might be outside the scope of this PR)
Could this code be put in the logger and reused?

@AaronH88
Copy link
Contributor

Great work on this, I am imagining using this now and I am throwing out ideas,

First idea, lets change RECEPTOR_PAYLOAD_DEBUG to RECEPTOR_PAYLOAD_TRACE_LEVEL which will expect an int Now that we have different "levels" for tracing, lets build the debug message in a switch case where the higher the int number results in more of the payload data being printed.

Imagine level 1 we just print something like "Unix message" , "Network message". Level 2 is like the full payload, which you have, Then Level 3 could be the payload plus the work unit id. (which you got!)

Let me know what you think.

The next thing to think about is code reusability (might be outside the scope of this PR) Could this code be put in the logger and reused?

Thinking more on this, in my example maybe we should switch the level 2 and 3 😂

@@ -62,6 +62,27 @@ Log level
- Error
- string

Add payload debuging using `RECEPTOR_PAYLOAD_DEBUG=int` envorment variable and using log level debug.
Copy link
Contributor

Choose a reason for hiding this comment

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

The Env var is called RECEPTOR_PAYLOAD_TRACE_LEVEL not this

Copy link
Contributor

Choose a reason for hiding this comment

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

In general I think it would be a good idea to replace everywhere where you have the word "debug" with the word "trace"

:header-rows: 1
:widths: auto

* - Debug level
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest "Tracing Level"

Copy link

sonarcloud bot commented Sep 27, 2024

Copy link
Contributor

@AaronH88 AaronH88 left a comment

Choose a reason for hiding this comment

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

lgtm!

@matoval matoval merged commit a6ed2b7 into ansible:devel Sep 27, 2024
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants