log: support multiple log entries under one path scope

fix issue #1044

Signed-off-by: Tw <tw19881113@gmail.com>

caddyhttp/log/log.go

@@ -21,7 +21,7 @@ func init() {
 // Logger is a basic request logging middleware.
 type Logger struct {
 	Next      httpserver.Handler
-	Rules     []Rule
+	Rules     []*Rule
 	ErrorFunc func(http.ResponseWriter, *http.Request, int) // failover error handler
 }
 
@@ -52,8 +52,10 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 				status = 0
 			}
 
-			// Write log entry
+			// Write log entries
-			rule.Log.Println(rep.Replace(rule.Format))
+			for _, e := range rule.Entries {
+				e.Log.Println(rep.Replace(e.Format))
+			}
 
 			return status, err
 		}
@@ -61,9 +63,8 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 	return l.Next.ServeHTTP(w, r)
 }
 
-// Rule configures the logging middleware.
+// Entry represents a log entry under a path scope
-type Rule struct {
+type Entry struct {
-	PathScope  string
 	OutputFile string
 	Format     string
 	Log        *log.Logger
@@ -71,6 +72,12 @@ type Rule struct {
 	file       *os.File // if logging to a file that needs to be closed
 }
 
+// Rule configures the logging middleware.
+type Rule struct {
+	PathScope string
+	Entries   []*Entry
+}
+
 const (
 	// DefaultLogFilename is the default log filename.
 	DefaultLogFilename = "access.log"

caddyhttp/log/log_test.go

@@ -26,12 +26,14 @@ func TestLoggedStatus(t *testing.T) {
 	var next erroringMiddleware
 	rule := Rule{
 		PathScope: "/",
+		Entries: []*Entry{{
 			Format: DefaultLogFormat + " {testval}",
 			Log:    log.New(&f, "", 0),
+		}},
 	}
 
 	logger := Logger{
-		Rules: []Rule{rule},
+		Rules: []*Rule{&rule},
 		Next:  next,
 	}
 
@@ -65,11 +67,13 @@ func TestLoggedStatus(t *testing.T) {
 func TestLogRequestBody(t *testing.T) {
 	var got bytes.Buffer
 	logger := Logger{
-		Rules: []Rule{{
+		Rules: []*Rule{{
 			PathScope: "/",
+			Entries: []*Entry{{
 				Format: "{request_body}",
 				Log:    log.New(&got, "", 0),
 			}},
+		}},
 		Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
 			// drain up body
 			ioutil.ReadAll(r.Body)

caddyhttp/log/setup.go

@@ -20,39 +20,41 @@ func setup(c *caddy.Controller) error {
 
 	// Open the log files for writing when the server starts
 	c.OnStartup(func() error {
-		for i := 0; i < len(rules); i++ {
+		for _, rule := range rules {
+			for _, entry := range rule.Entries {
 				var err error
 				var writer io.Writer
 
-			if rules[i].OutputFile == "stdout" {
+				if entry.OutputFile == "stdout" {
 					writer = os.Stdout
-			} else if rules[i].OutputFile == "stderr" {
+				} else if entry.OutputFile == "stderr" {
 					writer = os.Stderr
-			} else if rules[i].OutputFile == "syslog" {
+				} else if entry.OutputFile == "syslog" {
 					writer, err = gsyslog.NewLogger(gsyslog.LOG_INFO, "LOCAL0", "caddy")
 					if err != nil {
 						return err
 					}
 				} else {
-				err := os.MkdirAll(filepath.Dir(rules[i].OutputFile), 0744)
+					err := os.MkdirAll(filepath.Dir(entry.OutputFile), 0744)
 					if err != nil {
 						return err
 					}
-				file, err := os.OpenFile(rules[i].OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
+					file, err := os.OpenFile(entry.OutputFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
 					if err != nil {
 						return err
 					}
-				if rules[i].Roller != nil {
+					if entry.Roller != nil {
 						file.Close()
-					rules[i].Roller.Filename = rules[i].OutputFile
+						entry.Roller.Filename = entry.OutputFile
-					writer = rules[i].Roller.GetLogWriter()
+						writer = entry.Roller.GetLogWriter()
 					} else {
-					rules[i].file = file
+						entry.file = file
 						writer = file
 					}
 				}
 
-			rules[i].Log = log.New(writer, "", 0)
+				entry.Log = log.New(writer, "", 0)
+			}
 		}
 
 		return nil
@@ -61,8 +63,10 @@ func setup(c *caddy.Controller) error {
 	// When server stops, close any open log files
 	c.OnShutdown(func() error {
 		for _, rule := range rules {
-			if rule.file != nil {
+			for _, entry := range rule.Entries {
-				rule.file.Close()
+				if entry.file != nil {
+					entry.file.Close()
+				}
 			}
 		}
 		return nil
@@ -75,8 +79,8 @@ func setup(c *caddy.Controller) error {
 	return nil
 }
 
-func logParse(c *caddy.Controller) ([]Rule, error) {
+func logParse(c *caddy.Controller) ([]*Rule, error) {
-	var rules []Rule
+	var rules []*Rule
 
 	for c.Next() {
 		args := c.RemainingArgs()
@@ -103,16 +107,14 @@ func logParse(c *caddy.Controller) ([]Rule, error) {
 		}
 		if len(args) == 0 {
 			// Nothing specified; use defaults
-			rules = append(rules, Rule{
+			rules = appendEntry(rules, "/", &Entry{
-				PathScope:  "/",
 				OutputFile: DefaultLogFilename,
 				Format:     DefaultLogFormat,
 				Roller:     logRoller,
 			})
 		} else if len(args) == 1 {
 			// Only an output file specified
-			rules = append(rules, Rule{
+			rules = appendEntry(rules, "/", &Entry{
-				PathScope:  "/",
 				OutputFile: args[0],
 				Format:     DefaultLogFormat,
 				Roller:     logRoller,
@@ -133,8 +135,7 @@ func logParse(c *caddy.Controller) ([]Rule, error) {
 				}
 			}
 
-			rules = append(rules, Rule{
+			rules = appendEntry(rules, args[0], &Entry{
-				PathScope:  args[0],
 				OutputFile: args[1],
 				Format:     format,
 				Roller:     logRoller,
@@ -144,3 +145,19 @@ func logParse(c *caddy.Controller) ([]Rule, error) {
 
 	return rules, nil
 }
+
+func appendEntry(rules []*Rule, pathScope string, entry *Entry) []*Rule {
+	for _, rule := range rules {
+		if rule.PathScope == pathScope {
+			rule.Entries = append(rule.Entries, entry)
+			return rules
+		}
+	}
+
+	rules = append(rules, &Rule{
+		PathScope: pathScope,
+		Entries:   []*Entry{entry},
+	})
+
+	return rules
+}

caddyhttp/log/setup_test.go

@@ -29,14 +29,15 @@ func TestSetup(t *testing.T) {
 	if myHandler.Rules[0].PathScope != "/" {
 		t.Errorf("Expected / as the default PathScope")
 	}
-	if myHandler.Rules[0].OutputFile != DefaultLogFilename {
+	if myHandler.Rules[0].Entries[0].OutputFile != DefaultLogFilename {
 		t.Errorf("Expected %s as the default OutputFile", DefaultLogFilename)
 	}
-	if myHandler.Rules[0].Format != DefaultLogFormat {
+	if myHandler.Rules[0].Entries[0].Format != DefaultLogFormat {
 		t.Errorf("Expected %s as the default Log Format", DefaultLogFormat)
 	}
-	if myHandler.Rules[0].Roller != nil {
+	if myHandler.Rules[0].Entries[0].Roller != nil {
-		t.Errorf("Expected Roller to be nil, got: %v", *myHandler.Rules[0].Roller)
+		t.Errorf("Expected Roller to be nil, got: %v",
+			*myHandler.Rules[0].Entries[0].Roller)
 	}
 	if !httpserver.SameNext(myHandler.Next, httpserver.EmptyNext) {
 		t.Error("'Next' field of handler was not set properly")
@@ -52,56 +53,77 @@ func TestLogParse(t *testing.T) {
 	}{
 		{`log`, false, []Rule{{
 			PathScope: "/",
+			Entries: []*Entry{{
 				OutputFile: DefaultLogFilename,
 				Format:     DefaultLogFormat,
+			}},
 		}}},
 		{`log log.txt`, false, []Rule{{
 			PathScope: "/",
+			Entries: []*Entry{{
 				OutputFile: "log.txt",
 				Format:     DefaultLogFormat,
+			}},
 		}}},
 		{`log /api log.txt`, false, []Rule{{
 			PathScope: "/api",
+			Entries: []*Entry{{
 				OutputFile: "log.txt",
 				Format:     DefaultLogFormat,
+			}},
 		}}},
 		{`log /serve stdout`, false, []Rule{{
 			PathScope: "/serve",
+			Entries: []*Entry{{
 				OutputFile: "stdout",
 				Format:     DefaultLogFormat,
+			}},
 		}}},
 		{`log /myapi log.txt {common}`, false, []Rule{{
 			PathScope: "/myapi",
+			Entries: []*Entry{{
 				OutputFile: "log.txt",
 				Format:     CommonLogFormat,
+			}},
 		}}},
 		{`log /test accesslog.txt {combined}`, false, []Rule{{
 			PathScope: "/test",
+			Entries: []*Entry{{
 				OutputFile: "accesslog.txt",
 				Format:     CombinedLogFormat,
+			}},
 		}}},
 		{`log /api1 log.txt
 		  log /api2 accesslog.txt {combined}`, false, []Rule{{
 			PathScope: "/api1",
+			Entries: []*Entry{{
 				OutputFile: "log.txt",
 				Format:     DefaultLogFormat,
+			}},
 		}, {
 			PathScope: "/api2",
+			Entries: []*Entry{{
 				OutputFile: "accesslog.txt",
 				Format:     CombinedLogFormat,
+			}},
 		}}},
 		{`log /api3 stdout {host}
 		  log /api4 log.txt {when}`, false, []Rule{{
 			PathScope: "/api3",
+			Entries: []*Entry{{
 				OutputFile: "stdout",
 				Format:     "{host}",
+			}},
 		}, {
 			PathScope: "/api4",
+			Entries: []*Entry{{
 				OutputFile: "log.txt",
 				Format:     "{when}",
+			}},
 		}}},
 		{`log access.log { rotate { size 2 age 10 keep 3 } }`, false, []Rule{{
 			PathScope: "/",
+			Entries: []*Entry{{
 				OutputFile: "access.log",
 				Format:     DefaultLogFormat,
 				Roller: &httpserver.LogRoller{
@@ -110,6 +132,18 @@ func TestLogParse(t *testing.T) {
 					MaxBackups: 3,
 					LocalTime:  true,
 				},
+			}},
+		}}},
+		{`log / stdout {host}
+		  log / log.txt {when}`, false, []Rule{{
+			PathScope: "/",
+			Entries: []*Entry{{
+				OutputFile: "stdout",
+				Format:     "{host}",
+			}, {
+				OutputFile: "log.txt",
+				Format:     "{when}",
+			}},
 		}}},
 	}
 	for i, test := range tests {
@@ -132,39 +166,46 @@ func TestLogParse(t *testing.T) {
 					i, j, test.expectedLogRules[j].PathScope, actualLogRule.PathScope)
 			}
 
-			if actualLogRule.OutputFile != test.expectedLogRules[j].OutputFile {
+			if got, expect := len(actualLogRule.Entries), len(test.expectedLogRules[j].Entries); got != expect {
-				t.Errorf("Test %d expected %dth LogRule OutputFile to be  %s  , but got %s",
+				t.Fatalf("Test %d expected %dth LogRule with %d no of Log entries, but got %d ",
-					i, j, test.expectedLogRules[j].OutputFile, actualLogRule.OutputFile)
+					i, j, expect, got)
 			}
 
-			if actualLogRule.Format != test.expectedLogRules[j].Format {
+			for k, actualEntry := range actualLogRule.Entries {
+				if actualEntry.OutputFile != test.expectedLogRules[j].Entries[k].OutputFile {
+					t.Errorf("Test %d expected %dth LogRule OutputFile to be  %s  , but got %s",
+						i, j, test.expectedLogRules[j].Entries[k].OutputFile, actualEntry.OutputFile)
+				}
+
+				if actualEntry.Format != test.expectedLogRules[j].Entries[k].Format {
 					t.Errorf("Test %d expected %dth LogRule Format to be  %s  , but got %s",
-					i, j, test.expectedLogRules[j].Format, actualLogRule.Format)
+						i, j, test.expectedLogRules[j].Entries[k].Format, actualEntry.Format)
 				}
-			if actualLogRule.Roller != nil && test.expectedLogRules[j].Roller == nil || actualLogRule.Roller == nil && test.expectedLogRules[j].Roller != nil {
+				if actualEntry.Roller != nil && test.expectedLogRules[j].Entries[k].Roller == nil || actualEntry.Roller == nil && test.expectedLogRules[j].Entries[k].Roller != nil {
 					t.Fatalf("Test %d expected %dth LogRule Roller to be %v, but got %v",
-					i, j, test.expectedLogRules[j].Roller, actualLogRule.Roller)
+						i, j, test.expectedLogRules[j].Entries[k].Roller, actualEntry.Roller)
 				}
-			if actualLogRule.Roller != nil && test.expectedLogRules[j].Roller != nil {
+				if actualEntry.Roller != nil && test.expectedLogRules[j].Entries[k].Roller != nil {
-				if actualLogRule.Roller.Filename != test.expectedLogRules[j].Roller.Filename {
+					if actualEntry.Roller.Filename != test.expectedLogRules[j].Entries[k].Roller.Filename {
 						t.Fatalf("Test %d expected %dth LogRule Roller Filename to be %s, but got %s",
-						i, j, test.expectedLogRules[j].Roller.Filename, actualLogRule.Roller.Filename)
+							i, j, test.expectedLogRules[j].Entries[k].Roller.Filename, actualEntry.Roller.Filename)
 					}
-				if actualLogRule.Roller.MaxAge != test.expectedLogRules[j].Roller.MaxAge {
+					if actualEntry.Roller.MaxAge != test.expectedLogRules[j].Entries[k].Roller.MaxAge {
 						t.Fatalf("Test %d expected %dth LogRule Roller MaxAge to be %d, but got %d",
-						i, j, test.expectedLogRules[j].Roller.MaxAge, actualLogRule.Roller.MaxAge)
+							i, j, test.expectedLogRules[j].Entries[k].Roller.MaxAge, actualEntry.Roller.MaxAge)
 					}
-				if actualLogRule.Roller.MaxBackups != test.expectedLogRules[j].Roller.MaxBackups {
+					if actualEntry.Roller.MaxBackups != test.expectedLogRules[j].Entries[k].Roller.MaxBackups {
 						t.Fatalf("Test %d expected %dth LogRule Roller MaxBackups to be %d, but got %d",
-						i, j, test.expectedLogRules[j].Roller.MaxBackups, actualLogRule.Roller.MaxBackups)
+							i, j, test.expectedLogRules[j].Entries[k].Roller.MaxBackups, actualEntry.Roller.MaxBackups)
 					}
-				if actualLogRule.Roller.MaxSize != test.expectedLogRules[j].Roller.MaxSize {
+					if actualEntry.Roller.MaxSize != test.expectedLogRules[j].Entries[k].Roller.MaxSize {
 						t.Fatalf("Test %d expected %dth LogRule Roller MaxSize to be %d, but got %d",
-						i, j, test.expectedLogRules[j].Roller.MaxSize, actualLogRule.Roller.MaxSize)
+							i, j, test.expectedLogRules[j].Entries[k].Roller.MaxSize, actualEntry.Roller.MaxSize)
 					}
-				if actualLogRule.Roller.LocalTime != test.expectedLogRules[j].Roller.LocalTime {
+					if actualEntry.Roller.LocalTime != test.expectedLogRules[j].Entries[k].Roller.LocalTime {
 						t.Fatalf("Test %d expected %dth LogRule Roller LocalTime to be %t, but got %t",
-						i, j, test.expectedLogRules[j].Roller.LocalTime, actualLogRule.Roller.LocalTime)
+							i, j, test.expectedLogRules[j].Entries[k].Roller.LocalTime, actualEntry.Roller.LocalTime)
+					}
 				}
 			}
 		}