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
21 changes: 21 additions & 0 deletions docs/source/user_guide/configuration_options.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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"


.. list-table:: RECEPTOR_PAYLOAD_DEBUG options
: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"

- Description
* - 0
- No payload debug log
* - 1
- Log connection type
* - 2
- Log connection type and work unit id
* - 3
- Log connection type, work unit id and payload

**Warning: Payload Debugging May Expose Sensitive Data**

Please be aware that using payload debugging can potentially reveal sensitive information. This includes, but is not limited to, personal data, authentication tokens, and system configurations. Ensure that you only use debugging tools in a secure environment and avoid sharing debug output with unauthorized users. Always follow your organization's data protection policies when handling sensitive information. Proceed with caution!

.. code-block:: yaml

log-level:
Expand Down
36 changes: 34 additions & 2 deletions pkg/controlsvc/controlsvc.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package controlsvc

import (
"bufio"
"context"
"crypto/tls"
"encoding/json"
Expand All @@ -14,6 +15,7 @@
"os"
"reflect"
"runtime"
"strconv"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -122,8 +124,38 @@
if err := s.WriteMessage(message); err != nil {
return err
}
if _, err := io.Copy(out, s.conn); err != nil {
return err
payloadDebug, _ := strconv.Atoi(os.Getenv("RECEPTOR_PAYLOAD_TRACE_LEVEL"))

if payloadDebug != 0 {
var connectionType string
var payload string
if s.conn.LocalAddr().Network() == "unix" {
connectionType = "unix socket"
} else {
connectionType = "network connection"

Check warning on line 135 in pkg/controlsvc/controlsvc.go

View check run for this annotation

Codecov / codecov/patch

pkg/controlsvc/controlsvc.go#L130-L135

Added lines #L130 - L135 were not covered by tests
}
reader := bufio.NewReader(s.conn)

Check warning on line 137 in pkg/controlsvc/controlsvc.go

View check run for this annotation

Codecov / codecov/patch

pkg/controlsvc/controlsvc.go#L137

Added line #L137 was not covered by tests

for {
response, err := reader.ReadString('\n')
if err != nil {
if err.Error() != "EOF" {
MainInstance.nc.GetLogger().Error("Error reading from conn: %v \n", err)

Check warning on line 143 in pkg/controlsvc/controlsvc.go

View check run for this annotation

Codecov / codecov/patch

pkg/controlsvc/controlsvc.go#L139-L143

Added lines #L139 - L143 were not covered by tests
}

break

Check warning on line 146 in pkg/controlsvc/controlsvc.go

View check run for this annotation

Codecov / codecov/patch

pkg/controlsvc/controlsvc.go#L146

Added line #L146 was not covered by tests
}
payload += response

Check warning on line 148 in pkg/controlsvc/controlsvc.go

View check run for this annotation

Codecov / codecov/patch

pkg/controlsvc/controlsvc.go#L148

Added line #L148 was not covered by tests
}

MainInstance.nc.GetLogger().DebugPayload(payloadDebug, payload, "", connectionType)
if _, err := out.Write([]byte(payload)); err != nil {
return err

Check warning on line 153 in pkg/controlsvc/controlsvc.go

View check run for this annotation

Codecov / codecov/patch

pkg/controlsvc/controlsvc.go#L151-L153

Added lines #L151 - L153 were not covered by tests
}
} else {
if _, err := io.Copy(out, s.conn); err != nil {
return err
}
}

return nil
Expand Down
27 changes: 27 additions & 0 deletions pkg/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,33 @@ func (rl *ReceptorLogger) Debug(format string, v ...interface{}) {
rl.Log(DebugLevel, format, v...)
}

// Debug payload data.
func (rl *ReceptorLogger) DebugPayload(payloadDebug int, payload string, workUnitID string, connectionType string) {
var payloadMessage string
var workunitIDMessage string
var connectionTypeMessage string
switch payloadDebug {
case 3:
AaronH88 marked this conversation as resolved.
Show resolved Hide resolved
payloadMessage = fmt.Sprintf(" with a payload of: %s", payload)

fallthrough
case 2:
if workUnitID != "" {
workunitIDMessage = fmt.Sprintf(" with work unit %s", workUnitID)
} else {
workunitIDMessage = ", work unit not created yet"
}

fallthrough
case 1:
if connectionType != "" {
connectionTypeMessage = fmt.Sprintf("Reading from %s", connectionType)
}
default:
}
rl.Debug(fmt.Sprintf("PACKET TRACING ENABLED: %s%s%s", connectionTypeMessage, workunitIDMessage, payloadMessage)) //nolint:govet
}

// SanitizedDebug contains extra information helpful to developers.
func (rl *ReceptorLogger) SanitizedDebug(format string, v ...interface{}) {
rl.SanitizedLog(DebugLevel, format, v...)
Expand Down
52 changes: 52 additions & 0 deletions pkg/logger/logger_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package logger_test

import (
"bytes"
"fmt"
"os"
"testing"

"github.com/ansible/receptor/pkg/logger"
Expand Down Expand Up @@ -68,3 +70,53 @@ func TestLogLevelToNameWithError(t *testing.T) {
t.Error("should have error")
}
}

func TestDebugPayload(t *testing.T) {
logFilePath := "/tmp/test-output"
logger.SetGlobalLogLevel(4)
receptorLogger := logger.NewReceptorLogger("testDebugPayload")
logFile, err := os.OpenFile(logFilePath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o600)
if err != nil {
t.Error("error creating test-output file")
}

payload := "Testing debugPayload"
workUnitID := "1234"
connectionType := "unix socket"

debugPayloadTestCases := []struct {
name string
debugPayload int
payload string
workUnitID string
connectionType string
expectedLog string
}{
{name: "debugPayload no log", debugPayload: 0, payload: "", workUnitID: "", connectionType: "", expectedLog: ""},
{name: "debugPayload log level 1", debugPayload: 1, payload: "", workUnitID: "", connectionType: connectionType, expectedLog: fmt.Sprintf("PACKET TRACING ENABLED: Reading from %v", connectionType)},
{name: "debugPayload log level 2 with workUnitID", debugPayload: 2, payload: "", workUnitID: workUnitID, connectionType: connectionType, expectedLog: fmt.Sprintf("PACKET TRACING ENABLED: Reading from %v with work unit %v", connectionType, workUnitID)},
{name: "debugPayload log level 2 without workUnitID", debugPayload: 2, payload: "", workUnitID: "", connectionType: connectionType, expectedLog: fmt.Sprintf("PACKET TRACING ENABLED: Reading from %v", connectionType)},
{name: "debugPayload log level 3 with workUnitID", debugPayload: 3, payload: payload, workUnitID: workUnitID, connectionType: connectionType, expectedLog: fmt.Sprintf("PACKET TRACING ENABLED: Reading from %v with work unit %v with a payload of: %v", connectionType, workUnitID, payload)},
{name: "debugPayload log level 3 without workUnitID", debugPayload: 3, payload: payload, workUnitID: "", connectionType: connectionType, expectedLog: fmt.Sprintf("PACKET TRACING ENABLED: Reading from %v, work unit not created yet with a payload of: %v", connectionType, payload)},
{name: "debugPayload log level 3 without workUnitID and payload is new line", debugPayload: 3, payload: "\n", workUnitID: "", connectionType: connectionType, expectedLog: fmt.Sprintf("PACKET TRACING ENABLED: Reading from %v, work unit not created yet with a payload of: %v", connectionType, "\n")},
{name: "debugPayload log level 3 without workUnitID or payload", debugPayload: 3, payload: "", workUnitID: "", connectionType: connectionType, expectedLog: fmt.Sprintf("PACKET TRACING ENABLED: Reading from %v, work unit not created yet with a payload of: %v", connectionType, "")},
}

for _, testCase := range debugPayloadTestCases {
t.Run(testCase.name, func(t *testing.T) {
receptorLogger.SetOutput(logFile)
receptorLogger.DebugPayload(testCase.debugPayload, testCase.payload, testCase.workUnitID, testCase.connectionType)

testOutput, err := os.ReadFile(logFilePath)
if err != nil {
t.Error("error reading test-output file")
}
if !bytes.Contains(testOutput, []byte(testCase.expectedLog)) {
t.Errorf("failed to log correctly, expected: %v got %v", testCase.expectedLog, string(testOutput))
}
if err := os.Truncate(logFilePath, 0); err != nil {
t.Errorf("failed to truncate: %v", err)
}
})
}
}
36 changes: 35 additions & 1 deletion pkg/workceptor/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@
package workceptor

import (
"bufio"
"context"
"flag"
"fmt"
"io"
"os"
"os/exec"
"os/signal"
"path"
"strconv"
"strings"
"sync"
"syscall"
Expand Down Expand Up @@ -112,7 +115,38 @@
if err != nil {
return err
}
cmd.Stdin = stdin
payloadDebug, _ := strconv.Atoi(os.Getenv("RECEPTOR_PAYLOAD_TRACE_LEVEL"))

Check warning on line 118 in pkg/workceptor/command.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/command.go#L118

Added line #L118 was not covered by tests

if payloadDebug != 0 {
splitUnitDir := strings.Split(unitdir, "/")
workUnitID := splitUnitDir[len(splitUnitDir)-1]
stdinStream, err := cmd.StdinPipe()
if err != nil {
return err

Check warning on line 125 in pkg/workceptor/command.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/command.go#L120-L125

Added lines #L120 - L125 were not covered by tests
}
var payload string
reader := bufio.NewReader(stdin)
if err != nil {
return err

Check warning on line 130 in pkg/workceptor/command.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/command.go#L127-L130

Added lines #L127 - L130 were not covered by tests
}

for {
response, err := reader.ReadString('\n')
if err != nil {
if err.Error() != "EOF" {
MainInstance.nc.GetLogger().Error("Error reading work unit %v stdin: %v\n", workUnitID, err)

Check warning on line 137 in pkg/workceptor/command.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/command.go#L133-L137

Added lines #L133 - L137 were not covered by tests
}

break

Check warning on line 140 in pkg/workceptor/command.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/command.go#L140

Added line #L140 was not covered by tests
}
payload += response

Check warning on line 142 in pkg/workceptor/command.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/command.go#L142

Added line #L142 was not covered by tests
}

MainInstance.nc.GetLogger().DebugPayload(payloadDebug, payload, workUnitID, "")
io.WriteString(stdinStream, payload)
} else {
cmd.Stdin = stdin

Check warning on line 148 in pkg/workceptor/command.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/command.go#L145-L148

Added lines #L145 - L148 were not covered by tests
}
stdout, err := os.OpenFile(path.Join(unitdir, "stdout"), os.O_CREATE+os.O_WRONLY+os.O_SYNC, 0o600)
if err != nil {
return err
Expand Down
23 changes: 23 additions & 0 deletions pkg/workceptor/stdio_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
"io"
"os"
"path"
"strconv"
"strings"
"sync"
)

Expand Down Expand Up @@ -111,6 +113,7 @@
// STDinReader reads from a stdin file and provides a Done function.
type STDinReader struct {
reader FileReadCloser
workUnit string
lasterr error
doneChan chan struct{}
doneOnce sync.Once
Expand All @@ -120,6 +123,8 @@

// NewStdinReader allocates a new stdinReader, which reads from a stdin file and provides a Done function.
func NewStdinReader(fs FileSystemer, unitdir string) (*STDinReader, error) {
splitUnitDir := strings.Split(unitdir, "/")
workUnitID := splitUnitDir[len(splitUnitDir)-1]
stdinpath := path.Join(unitdir, "stdin")
stat, err := fs.Stat(stdinpath)
if err != nil {
Expand All @@ -135,6 +140,7 @@

return &STDinReader{
reader: reader,
workUnit: workUnitID,
lasterr: nil,
doneChan: make(chan struct{}),
doneOnce: sync.Once{},
Expand All @@ -143,6 +149,23 @@

// Read reads data from the stdout file, implementing io.Reader.
func (sr *STDinReader) Read(p []byte) (n int, err error) {
payloadDebug, _ := strconv.Atoi(os.Getenv("RECEPTOR_PAYLOAD_TRACE_LEVEL"))

if payloadDebug != 0 {
isNotEmpty := func() bool {
for _, v := range p {
if v != 0 {
return true

Check warning on line 158 in pkg/workceptor/stdio_utils.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/stdio_utils.go#L155-L158

Added lines #L155 - L158 were not covered by tests
}
}

return false

Check warning on line 162 in pkg/workceptor/stdio_utils.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/stdio_utils.go#L162

Added line #L162 was not covered by tests
}()
if isNotEmpty {
payload := string(p)
MainInstance.nc.GetLogger().DebugPayload(payloadDebug, payload, sr.workUnit, "kube api")

Check warning on line 166 in pkg/workceptor/stdio_utils.go

View check run for this annotation

Codecov / codecov/patch

pkg/workceptor/stdio_utils.go#L164-L166

Added lines #L164 - L166 were not covered by tests
}
}
n, err = sr.reader.Read(p)
if err != nil {
sr.lasterr = err
Expand Down