Configurable log destination (#1258)

* COnfigurable log destination

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* Custom writer impl

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* fix tests

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* fix e2e tests

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* fix flaky mar tests

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* adding e2e for log destination

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* adding tests and change logic to create log file

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* update e2e tests

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

* review comments

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>

---------

Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com>
Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
This commit is contained in:
Pravin Pushkar 2023-04-24 11:30:46 +05:30 committed by GitHub
parent 4e3ae81df5
commit 7abf871ce0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 315 additions and 51 deletions

View File

@ -499,13 +499,17 @@ func executeRun(runFilePath string, apps []runfileconfig.App) (bool, error) {
}
// Combined multiwriter for logs.
var appDaprdWriter io.Writer
// appLogWriterCloser is used when app command is present.
var appLogWriterCloser io.WriteCloser
daprdLogWriterCloser := app.DaprdLogWriteCloser
// appLogWriter is used when app command is present.
var appLogWriter io.Writer
// A custom writer used for trimming ASCII color codes from logs when writing to files.
var customAppLogWriter io.Writer
daprdLogWriterCloser := getLogWriter(app.DaprdLogWriteCloser, app.DaprdLogDestination)
if len(runConfig.Command) == 0 {
print.StatusEvent(os.Stdout, print.LogWarning, "No application command found for app %q present in %s", runConfig.AppID, runFilePath)
appDaprdWriter = app.DaprdLogWriteCloser
appLogWriterCloser = app.DaprdLogWriteCloser
appDaprdWriter = getAppDaprdWriter(app, true)
appLogWriter = app.DaprdLogWriteCloser
} else {
err = app.CreateAppLogFile()
if err != nil {
@ -513,14 +517,13 @@ func executeRun(runFilePath string, apps []runfileconfig.App) (bool, error) {
exitWithError = true
break
}
appDaprdWriter = io.MultiWriter(app.AppLogWriteCloser, app.DaprdLogWriteCloser)
appLogWriterCloser = app.AppLogWriteCloser
appDaprdWriter = getAppDaprdWriter(app, false)
appLogWriter = getLogWriter(app.AppLogWriteCloser, app.AppLogDestination)
}
customAppLogWriter = print.CustomLogWriter{W: appLogWriter}
runState, err := startDaprdAndAppProcesses(&runConfig, app.AppDirPath, sigCh,
daprdLogWriterCloser, daprdLogWriterCloser, appLogWriterCloser, appLogWriterCloser)
daprdLogWriterCloser, daprdLogWriterCloser, customAppLogWriter, customAppLogWriter)
if err != nil {
print.StatusEvent(os.Stdout, print.LogFailure, "Error starting Dapr and app (%q): %s", app.AppID, err.Error())
print.StatusEvent(appDaprdWriter, print.LogFailure, "Error starting Dapr and app (%q): %s", app.AppID, err.Error())
exitWithError = true
break
@ -569,6 +572,43 @@ func executeRun(runFilePath string, apps []runfileconfig.App) (bool, error) {
return exitWithError, closeError
}
// getAppDaprdWriter returns the writer for writing logs common to both daprd, app and stdout.
func getAppDaprdWriter(app runfileconfig.App, isAppCommandEmpty bool) io.Writer {
var appDaprdWriter io.Writer
if isAppCommandEmpty {
if app.DaprdLogDestination != runfileconfig.Console {
appDaprdWriter = io.MultiWriter(os.Stdout, app.DaprdLogWriteCloser)
} else {
appDaprdWriter = os.Stdout
}
} else {
if app.AppLogDestination != runfileconfig.Console && app.DaprdLogDestination != runfileconfig.Console {
appDaprdWriter = io.MultiWriter(app.AppLogWriteCloser, app.DaprdLogWriteCloser, os.Stdout)
} else if app.AppLogDestination != runfileconfig.Console {
appDaprdWriter = io.MultiWriter(app.AppLogWriteCloser, os.Stdout)
} else if app.DaprdLogDestination != runfileconfig.Console {
appDaprdWriter = io.MultiWriter(app.DaprdLogWriteCloser, os.Stdout)
} else {
appDaprdWriter = os.Stdout
}
}
return appDaprdWriter
}
// getLogWriter returns the log writer based on the log destination.
func getLogWriter(fileLogWriterCloser io.WriteCloser, logDestination runfileconfig.LogDestType) io.Writer {
var logWriter io.Writer
switch logDestination {
case runfileconfig.Console:
logWriter = os.Stdout
case runfileconfig.File:
logWriter = fileLogWriterCloser
case runfileconfig.FileAndConsole:
logWriter = io.MultiWriter(os.Stdout, fileLogWriterCloser)
}
return logWriter
}
func logInformationalStatusToStdout(app runfileconfig.App) {
print.InfoStatusEvent(os.Stdout, "Started Dapr with app id %q. HTTP Port: %d. gRPC Port: %d",
app.AppID, app.RunConfig.HTTPPort, app.RunConfig.GRPCPort)
@ -759,25 +799,14 @@ func startAppProcess(runConfig *standalone.RunConfig, runE *runExec.RunExec,
outScanner := bufio.NewScanner(stdOutPipe)
go func() {
for errScanner.Scan() {
if runE.AppCMD.ErrorWriter == os.Stderr {
// Add color and prefix to log and output to Stderr.
fmt.Fprintln(runE.AppCMD.ErrorWriter, print.Blue(fmt.Sprintf("== APP == %s", errScanner.Text())))
} else {
// Directly output app logs to the error writer.
fmt.Fprintln(runE.AppCMD.ErrorWriter, errScanner.Text())
}
fmt.Fprintln(runE.AppCMD.ErrorWriter, print.Blue(fmt.Sprintf("== APP - %s == %s", runE.AppID,
errScanner.Text())))
}
}()
go func() {
for outScanner.Scan() {
if runE.AppCMD.OutputWriter == os.Stdout {
// Add color and prefix to log and output to Stdout.
fmt.Fprintln(runE.AppCMD.OutputWriter, print.Blue(fmt.Sprintf("== APP == %s", outScanner.Text())))
} else {
// Directly output app logs to the output writer.
fmt.Fprintln(runE.AppCMD.OutputWriter, outScanner.Text())
}
fmt.Fprintln(runE.AppCMD.OutputWriter, print.Blue(fmt.Sprintf("== APP - %s == %s", runE.AppID, outScanner.Text())))
}
}()
@ -830,10 +859,8 @@ func startDaprdProcess(runConfig *standalone.RunConfig, runE *runExec.RunExec,
daprRunning <- false
return
}
go func() {
daprdErr := runE.DaprCMD.Command.Wait()
if daprdErr != nil {
runE.DaprCMD.CommandErr = daprdErr
print.StatusEvent(runE.DaprCMD.ErrorWriter, print.LogFailure, "The daprd process exited with error code: %s", daprdErr.Error())

View File

@ -18,6 +18,8 @@ import (
"fmt"
"io"
"os"
"reflect"
"regexp"
"runtime"
"sync"
"time"
@ -204,3 +206,37 @@ func logJSON(w io.Writer, status, message string) {
fmt.Fprintf(w, "%s\n", string(jsonBytes))
}
type CustomLogWriter struct {
W io.Writer
}
func (c CustomLogWriter) Write(p []byte) (int, error) {
write := func(w io.Writer, isStdIO bool) (int, error) {
b := p
if !isStdIO {
// below regex is used to replace the color codes from the logs collected in the log file.
reg := regexp.MustCompile("\x1b\\[[\\d;]+m")
b = reg.ReplaceAll(b, []byte(""))
}
n, err := w.Write(b)
if err != nil {
return n, err
}
if n != len(b) {
return n, io.ErrShortWrite
}
return len(b), nil
}
wIface := reflect.ValueOf(c.W).Interface()
switch wType := wIface.(type) {
case *os.File:
if wType == os.Stderr || wType == os.Stdout {
return write(c.W, true)
} else {
return write(c.W, false)
}
default:
return write(c.W, false)
}
}

View File

@ -14,6 +14,7 @@ limitations under the License.
package runfileconfig
import (
"fmt"
"io"
"os"
"path/filepath"
@ -22,7 +23,15 @@ import (
"github.com/dapr/cli/pkg/standalone"
)
type LogDestType string
const (
Console LogDestType = "console"
File LogDestType = "file"
FileAndConsole LogDestType = "fileAndConsole"
DefaultDaprdLogDest = File
DefaultAppLogDest = FileAndConsole
appLogFileNamePrefix = "app"
daprdLogFileNamePrefix = "daprd"
logFileExtension = ".log"
@ -45,6 +54,8 @@ type App struct {
DaprdLogFileName string
AppLogWriteCloser io.WriteCloser
DaprdLogWriteCloser io.WriteCloser
DaprdLogDestination LogDestType `yaml:"daprdLogDestination"`
AppLogDestination LogDestType `yaml:"appLogDestination"`
}
// Common represents the configuration options for the common section in the run file.
@ -61,7 +72,13 @@ func (a *App) GetLogsDir() string {
// CreateAppLogFile creates the log file, sets internal file handle
// and returns error if any.
func (a *App) CreateAppLogFile() error {
f, err := a.createLogFile(appLogFileNamePrefix)
var err error
var f *os.File
if a.AppLogDestination == Console {
f = os.Stdout
} else {
f, err = a.createLogFile(appLogFileNamePrefix)
}
if err == nil {
a.AppLogWriteCloser = f
a.AppLogFileName = f.Name()
@ -72,7 +89,13 @@ func (a *App) CreateAppLogFile() error {
// CreateDaprdLogFile creates the log file, sets internal file handle
// and returns error if any.
func (a *App) CreateDaprdLogFile() error {
f, err := a.createLogFile(daprdLogFileNamePrefix)
var err error
var f *os.File
if a.DaprdLogDestination == Console {
f = os.Stdout
} else {
f, err = a.createLogFile(daprdLogFileNamePrefix)
}
if err == nil {
a.DaprdLogWriteCloser = f
a.DaprdLogFileName = f.Name()
@ -102,3 +125,15 @@ func (a *App) CloseDaprdLogFile() error {
}
return nil
}
func (l LogDestType) String() string {
return string(l)
}
func (l LogDestType) IsValid() error {
switch l {
case Console, File, FileAndConsole:
return nil
}
return fmt.Errorf("invalid log destination type: %s", l)
}

View File

@ -91,11 +91,13 @@ func (a *RunFileConfig) GetApps(runFilePath string) ([]App, error) {
}
a.mergeCommonAndAppsSharedRunConfig()
a.mergeCommonAndAppsEnv()
// Resolve app ids if not provided in the run file.
err = a.setAppIDIfEmpty()
// Set and validates default fields in the run file.
err = a.setDefaultFields()
if err != nil {
return nil, err
}
return a.Apps, nil
}
@ -121,17 +123,44 @@ func (a *RunFileConfig) mergeCommonAndAppsSharedRunConfig() {
}
}
// setDefaultFields sets the default values for the fields that are not provided in the run file.
func (a *RunFileConfig) setDefaultFields() error {
for i := range a.Apps {
if err := a.setAppIDIfEmpty(&a.Apps[i]); err != nil {
return err
}
if err := a.setAndValidateLogDestination(&a.Apps[i]); err != nil {
return err
}
}
return nil
}
// Set AppID to the directory name of appDirPath.
// appDirPath is a mandatory field in the run file and at this point it is already validated and resolved to its absolute path.
func (a *RunFileConfig) setAppIDIfEmpty() error {
for i := range a.Apps {
if a.Apps[i].AppID == "" {
basePath, err := a.getBasePathFromAbsPath(a.Apps[i].AppDirPath)
if err != nil {
return err
}
a.Apps[i].AppID = basePath
func (a *RunFileConfig) setAppIDIfEmpty(app *App) error {
if app.AppID == "" {
basePath, err := a.getBasePathFromAbsPath(app.AppDirPath)
if err != nil {
return fmt.Errorf("error in setting the app id: %w", err)
}
app.AppID = basePath
}
return nil
}
// setAndValidateLogDestination sets the default log destination if not provided in the run file.
// It also validates the log destination if provided.
func (a *RunFileConfig) setAndValidateLogDestination(app *App) error {
if app.DaprdLogDestination == "" {
app.DaprdLogDestination = DefaultDaprdLogDest
} else if err := app.DaprdLogDestination.IsValid(); err != nil {
return err
}
if app.AppLogDestination == "" {
app.AppLogDestination = DefaultAppLogDest
} else if err := app.AppLogDestination.IsValid(); err != nil {
return err
}
return nil
}

View File

@ -29,6 +29,7 @@ var (
invalidRunFilePath2 = filepath.Join("..", "testdata", "runfileconfig", "test_run_config_empty_app_dir.yaml")
runFileForPrecedenceRule = filepath.Join("..", "testdata", "runfileconfig", "test_run_config_precedence_rule.yaml")
runFileForPrecedenceRuleDaprDir = filepath.Join("..", "testdata", "runfileconfig", "test_run_config_precedence_rule_dapr_dir.yaml")
runFileForLogDestination = filepath.Join("..", "testdata", "runfileconfig", "test_run_config_log_destination.yaml")
)
func TestRunConfigFile(t *testing.T) {
@ -224,6 +225,31 @@ func TestRunConfigFile(t *testing.T) {
})
}
})
t.Run("test log destination for daprd and apps", func(t *testing.T) {
config := RunFileConfig{}
apps, err := config.GetApps(runFileForLogDestination)
assert.NoError(t, err)
assert.Equal(t, 6, len(apps))
assert.Equal(t, "file", apps[0].DaprdLogDestination.String())
assert.Equal(t, "fileAndConsole", apps[0].AppLogDestination.String())
assert.Equal(t, "fileAndConsole", apps[1].DaprdLogDestination.String())
assert.Equal(t, "fileAndConsole", apps[1].AppLogDestination.String())
assert.Equal(t, "file", apps[2].DaprdLogDestination.String())
assert.Equal(t, "file", apps[2].AppLogDestination.String())
assert.Equal(t, "console", apps[3].DaprdLogDestination.String())
assert.Equal(t, "console", apps[3].AppLogDestination.String())
assert.Equal(t, "console", apps[4].DaprdLogDestination.String())
assert.Equal(t, "file", apps[4].AppLogDestination.String())
assert.Equal(t, "file", apps[5].DaprdLogDestination.String())
assert.Equal(t, "console", apps[5].AppLogDestination.String())
})
}
func TestGetBasePathFromAbsPath(t *testing.T) {

View File

@ -0,0 +1,25 @@
version: 1
common:
apps:
- appDirPath: ./webapp/
appID: app_1
- appDirPath: ./webapp/
appID: app_2
daprdLogDestination: fileAndConsole
appLogDestination: fileAndConsole
- appDirPath: ./webapp/
appID: app_3
daprdLogDestination: file
appLogDestination: file
- appDirPath: ./webapp/
appID: app_4
daprdLogDestination: console
appLogDestination: console
- appDirPath: ./webapp/
appID: app_5
daprdLogDestination: console
appLogDestination: file
- appDirPath: ./webapp/
appID: app_6
daprdLogDestination: file
appLogDestination: console

View File

@ -34,11 +34,12 @@ import (
)
type AppTestOutput struct {
appID string
appLogContents []string
daprdLogContent []string
baseLogDirPath string
appLogDoesNotExist bool
appID string
appLogContents []string
daprdLogContent []string
baseLogDirPath string
appLogDoesNotExist bool
daprdLogFileDoesNotExist bool
}
func TestRunWithTemplateFile(t *testing.T) {
@ -135,7 +136,7 @@ func TestRunWithTemplateFile(t *testing.T) {
appID: "processor",
baseLogDirPath: "../../apps/processor/.dapr/logs",
appLogContents: []string{
"Received metrics: {3}",
"Received metrics: {1}",
},
daprdLogContent: []string{
"http server is running on port 3510",
@ -149,7 +150,7 @@ func TestRunWithTemplateFile(t *testing.T) {
appLogContents: []string{
"DAPR_HTTP_PORT set to 3511",
"DAPR_HOST_ADD set to localhost",
"Metrics with ID 3 sent",
"Metrics with ID 1 sent",
},
daprdLogContent: []string{
"termination signal received: shutting down",
@ -311,6 +312,64 @@ func TestRunWithTemplateFile(t *testing.T) {
}
assertLogOutputForRunTemplateExec(t, appTestOutput)
})
t.Run("valid template file with app/daprd log destinations", func(t *testing.T) {
runFilePath := "../testdata/run-template-files/app_output_to_file_and_console.yaml"
t.Cleanup(func() {
// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
os.RemoveAll("../../apps/emit-metrics/.dapr/logs")
os.RemoveAll("../../apps/processor/.dapr/logs")
stopAllApps(t, runFilePath)
})
args := []string{
"-f", runFilePath,
}
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
output, err := cmdRunWithContext(ctx, "", args...)
t.Logf(output)
require.NoError(t, err, "run failed")
// App logs for processor app should not be printed to console and only written to file.
assert.NotContains(t, output, "== APP - processor")
// Daprd logs for processor app should only be printed to console and not written to file.
assert.Contains(t, output, "msg=\"all outstanding components processed\" app_id=processor")
// App logs for emit-metrics app should be printed to console and written to file.
assert.Contains(t, output, "== APP - emit-metrics")
// Daprd logs for emit-metrics app should only be written to file.
assert.NotContains(t, output, "msg=\"all outstanding components processed\" app_id=emit-metrics")
assert.Contains(t, output, "Received signal to stop Dapr and app processes. Shutting down Dapr and app processes.")
appTestOutput := AppTestOutput{
appID: "processor",
baseLogDirPath: "../../apps/processor/.dapr/logs",
appLogContents: []string{
"Received metrics: {1}",
},
daprdLogContent: []string{},
daprdLogFileDoesNotExist: true,
}
assertLogOutputForRunTemplateExec(t, appTestOutput)
appTestOutput = AppTestOutput{
appID: "emit-metrics",
baseLogDirPath: "../../apps/emit-metrics/.dapr/logs",
appLogContents: []string{
"DAPR_HTTP_PORT set to 3511",
"DAPR_HOST_ADD set to localhost",
"Metrics with ID 1 sent",
},
daprdLogContent: []string{
"termination signal received: shutting down",
"Exited Dapr successfully",
"Exited App successfully",
},
}
assertLogOutputForRunTemplateExec(t, appTestOutput)
})
}
func TestRunTemplateFileWithoutDaprInit(t *testing.T) {
@ -328,7 +387,7 @@ func TestRunTemplateFileWithoutDaprInit(t *testing.T) {
output, err := cmdRunWithContext(ctx, "", args...)
t.Logf(output)
require.Error(t, err, "run must fail")
assert.Contains(t, output, " Error starting Dapr and app (\"processor\"): fork/exec")
assert.Contains(t, output, "Error starting Dapr and app (\"processor\"): fork/exec")
assert.Contains(t, output, "daprd: no such file or directory")
})
}
@ -336,11 +395,12 @@ func TestRunTemplateFileWithoutDaprInit(t *testing.T) {
func assertLogOutputForRunTemplateExec(t *testing.T, appTestOutput AppTestOutput) {
// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
// This is true for the tests in this file.
daprdLogFileName, err := lookUpFileFullName(appTestOutput.baseLogDirPath, "daprd")
require.NoError(t, err, "failed to find daprd log file")
daprdLogPath := filepath.Join(appTestOutput.baseLogDirPath, daprdLogFileName)
readAndAssertLogFileContents(t, daprdLogPath, appTestOutput.daprdLogContent)
if !appTestOutput.daprdLogFileDoesNotExist {
daprdLogFileName, err := lookUpFileFullName(appTestOutput.baseLogDirPath, "daprd")
require.NoError(t, err, "failed to find daprd log file")
daprdLogPath := filepath.Join(appTestOutput.baseLogDirPath, daprdLogFileName)
readAndAssertLogFileContents(t, daprdLogPath, appTestOutput.daprdLogContent)
}
if appTestOutput.appLogDoesNotExist {
return
}
@ -378,4 +438,5 @@ func lookUpFileFullName(dirPath, partialFilename string) (string, error) {
func stopAllApps(t *testing.T, runfile string) {
_, err := cmdStopWithRunTemplate(runfile)
require.NoError(t, err, "failed to stop apps")
time.Sleep(5 * time.Second)
}

View File

@ -110,6 +110,7 @@ func getCLIPID(t *testing.T) string {
}
func verifyCLIPIDNotExist(t *testing.T, pid string) {
time.Sleep(5 * time.Second)
output, err := cmdList("")
require.NoError(t, err, "failed to list apps")
assert.NotContains(t, output, pid)

View File

@ -0,0 +1,14 @@
version: 1
apps:
- appDirPath: ../../../apps/processor/
appPort: 9081
daprHTTPPort: 3510
command: ["go","run", "app.go"]
appLogDestination: file
daprdLogDestination: console
- appID: emit-metrics
appDirPath: ../../../apps/emit-metrics/
daprHTTPPort: 3511
env:
DAPR_HOST_ADD: localhost
command: ["go","run", "app.go"]

View File

@ -4,9 +4,11 @@ apps:
appPort: 9081
daprHTTPPort: 3510
command: ["go","run", "app.go"]
appLogDestination: file
- appID: emit-metrics
appDirPath: ../../../apps/emit-metrics/
daprHTTPPort: 3511
env:
DAPR_HOST_ADD: localhost
command: ["go","run", "app.go"]
appLogDestination: file

View File

@ -4,9 +4,11 @@ apps:
appPort: 9081
daprHTTPPort: 3510
command: ["go","run", "app.go"]
appLogDestination: file
- appID: emit-metrics
appDirPath: ../../../apps/emit-metrics/
daprHTTPPort: 3511
appLogDestination: file
env:
DAPR_HOST_ADD: localhost
command: [""]

View File

@ -3,8 +3,10 @@ apps:
- appDirPath: ../../../apps/processor/
appPort: 9081
daprHTTPPort: 3510
appLogDestination: file
command: ["go","run", "app.go"]
- appID: emit-metrics
appDirPath: ../../../apps/emit-metrics/
daprHTTPPort: 3511 # required env var DAPR_HOST_ADD not set in the yaml file
appLogDestination: file
command: ["go","run", "app.go"]

View File

@ -9,9 +9,11 @@ apps:
appPort: 9081
daprHTTPPort: 3510
command: ["go","run", "app.go"]
appLogDestination: file
- appID: emit-metrics
appDirPath: ../../../apps/emit-metrics/
daprHTTPPort: 3511
appLogDestination: file
env:
DAPR_HOST_ADD: localhost

View File

@ -4,6 +4,7 @@ apps:
appPort: 9081
daprHTTPPort: 3510
command: ["go","run", "app.go"]
appLogDestination: file
- appID: emit-metrics
appDirPath: ../../../apps/emit-metrics/
daprHTTPPort: 3511
@ -11,3 +12,4 @@ apps:
DAPR_HOST_ADD: localhost
# Set wrong app name
command: ["go","run", "wrongappname.go"]
appLogDestination: file