Avoid using a map for log attributes

Having a map per log entry seemed heavier than necessary. These
attributes end up being sorted and serialized, so storing them in a map
doesn't add anything (there's no random access element). In SwarmKit,
they originate as a slice, so there's an unnecessary conversion to a map
and back.

This also fixes the sort comparator, which used to inefficiently split
the string on each comparison.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

api/server/httputils/write_log_stream.go

@@ -5,7 +5,6 @@ import (
 	"io"
 	"net/url"
 	"sort"
-	"strings"
 
 	"golang.org/x/net/context"
 
@@ -53,7 +52,8 @@ func WriteLogStream(ctx context.Context, w io.Writer, msgs <-chan *backend.LogMe
 		}
 		logLine := msg.Line
 		if config.Details {
-			logLine = append([]byte(stringAttrs(msg.Attrs)+" "), logLine...)
+			logLine = append(attrsByteSlice(msg.Attrs), ' ')
+			logLine = append(logLine, msg.Line...)
 		}
 		if config.Timestamps {
 			// TODO(dperny) the format is defined in
@@ -71,24 +71,26 @@ func WriteLogStream(ctx context.Context, w io.Writer, msgs <-chan *backend.LogMe
 	}
 }
 
-type byKey []string
+type byKey []backend.LogAttr
 
-func (s byKey) Len() int { return len(s) }
-func (s byKey) Less(i, j int) bool {
-	keyI := strings.Split(s[i], "=")
-	keyJ := strings.Split(s[j], "=")
-	return keyI[0] < keyJ[0]
-}
-func (s byKey) Swap(i, j int) {
-	s[i], s[j] = s[j], s[i]
-}
+func (b byKey) Len() int           { return len(b) }
+func (b byKey) Less(i, j int) bool { return b[i].Key < b[j].Key }
+func (b byKey) Swap(i, j int)      { b[i], b[j] = b[j], b[i] }
 
-func stringAttrs(a backend.LogAttributes) string {
-	var ss byKey
-	for k, v := range a {
-		k, v := url.QueryEscape(k), url.QueryEscape(v)
-		ss = append(ss, k+"="+v)
+func attrsByteSlice(a []backend.LogAttr) []byte {
+	// Note this sorts "a" in-place. That is fine here - nothing else is
+	// going to use Attrs or care about the order.
+	sort.Sort(byKey(a))
+
+	var ret []byte
+	for i, pair := range a {
+		k, v := url.QueryEscape(pair.Key), url.QueryEscape(pair.Value)
+		ret = append(ret, []byte(k)...)
+		ret = append(ret, '=')
+		ret = append(ret, []byte(v)...)
+		if i != len(a)-1 {
+			ret = append(ret, ',')
+		}
 	}
-	sort.Sort(ss)
-	return strings.Join(ss, ",")
+	return ret
 }

api/types/backend/backend.go

@@ -35,7 +35,7 @@ type LogMessage struct {
 	Line      []byte
 	Source    string
 	Timestamp time.Time
-	Attrs     LogAttributes
+	Attrs     []LogAttr
 	Partial   bool
 
 	// Err is an error associated with a message. Completeness of a message
@@ -44,9 +44,11 @@ type LogMessage struct {
 	Err error
 }
 
-// LogAttributes is used to hold the extra attributes available in the log message
-// Primarily used for converting the map type to string and sorting.
-type LogAttributes map[string]string
+// LogAttr is used to hold the extra attributes available in the log message.
+type LogAttr struct {
+	Key   string
+	Value string
+}
 
 // LogSelector is a list of services and tasks that should be returned as part
 // of a log stream. It is similar to swarmapi.LogSelector, with the difference

daemon/cluster/executor/container/controller.go

@@ -524,10 +524,12 @@ func (r *controller) Logs(ctx context.Context, publisher exec.LogPublisher, opti
 		}
 
 		// parse the details out of the Attrs map
-		attrs := []api.LogAttr{}
-		for k, v := range msg.Attrs {
-			attr := api.LogAttr{Key: k, Value: v}
-			attrs = append(attrs, attr)
+		var attrs []api.LogAttr
+		if len(msg.Attrs) != 0 {
+			attrs = make([]api.LogAttr, 0, len(msg.Attrs))
+			for _, attr := range msg.Attrs {
+				attrs = append(attrs, api.LogAttr{Key: attr.Key, Value: attr.Value})
+			}
 		}
 
 		if err := publisher.Publish(ctx, api.LogMessage{

daemon/cluster/services.go

@@ -458,22 +458,33 @@ func (c *Cluster) ServiceLogs(ctx context.Context, selector *backend.LogSelector
 			for _, msg := range subscribeMsg.Messages {
 				// make a new message
 				m := new(backend.LogMessage)
-				m.Attrs = make(backend.LogAttributes)
+				m.Attrs = make([]backend.LogAttr, 0, len(msg.Attrs)+3)
 				// add the timestamp, adding the error if it fails
 				m.Timestamp, err = gogotypes.TimestampFromProto(msg.Timestamp)
 				if err != nil {
 					m.Err = err
 				}
+
+				nodeKey := contextPrefix + ".node.id"
+				serviceKey := contextPrefix + ".service.id"
+				taskKey := contextPrefix + ".task.id"
+
 				// copy over all of the details
 				for _, d := range msg.Attrs {
-					m.Attrs[d.Key] = d.Value
+					switch d.Key {
+					case nodeKey, serviceKey, taskKey:
+						// we have the final say over context details (in case there
+						// is a conflict (if the user added a detail with a context's
+						// key for some reason))
+					default:
+						m.Attrs = append(m.Attrs, backend.LogAttr{Key: d.Key, Value: d.Value})
+					}
 				}
-				// we have the final say over context details (in case there
-				// is a conflict (if the user added a detail with a context's
-				// key for some reason))
-				m.Attrs[contextPrefix+".node.id"] = msg.Context.NodeID
-				m.Attrs[contextPrefix+".service.id"] = msg.Context.ServiceID
-				m.Attrs[contextPrefix+".task.id"] = msg.Context.TaskID
+				m.Attrs = append(m.Attrs,
+					backend.LogAttr{Key: nodeKey, Value: msg.Context.NodeID},
+					backend.LogAttr{Key: serviceKey, Value: msg.Context.ServiceID},
+					backend.LogAttr{Key: taskKey, Value: msg.Context.TaskID},
+				)
 
 				switch msg.Stream {
 				case swarmapi.LogStreamStdout:

daemon/logger/journald/read.go

@@ -157,6 +157,7 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/coreos/go-systemd/journal"
+	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/daemon/logger"
 )
 
@@ -213,14 +214,11 @@ drain:
 				source = "stdout"
 			}
 			// Retrieve the values of any variables we're adding to the journal.
-			attrs := make(map[string]string)
+			var attrs []backend.LogAttr
 			C.sd_journal_restart_data(j)
 			for C.get_attribute_field(j, &data, &length) > C.int(0) {
 				kv := strings.SplitN(C.GoStringN(data, C.int(length)), "=", 2)
-				attrs[kv[0]] = kv[1]
-			}
-			if len(attrs) == 0 {
-				attrs = nil
+				attrs = append(attrs, backend.LogAttr{Key: kv[0], Value: kv[1]})
 			}
 			// Send the log message.
 			logWatcher.Msg <- &logger.Message{

daemon/logger/jsonfilelog/read.go

@@ -12,6 +12,7 @@ import (
 	"golang.org/x/net/context"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/daemon/logger"
 	"github.com/docker/docker/daemon/logger/jsonfilelog/multireader"
 	"github.com/docker/docker/pkg/filenotify"
@@ -27,11 +28,18 @@ func decodeLogLine(dec *json.Decoder, l *jsonlog.JSONLog) (*logger.Message, erro
 	if err := dec.Decode(l); err != nil {
 		return nil, err
 	}
+	var attrs []backend.LogAttr
+	if len(l.Attrs) != 0 {
+		attrs = make([]backend.LogAttr, 0, len(l.Attrs))
+		for k, v := range l.Attrs {
+			attrs = append(attrs, backend.LogAttr{Key: k, Value: v})
+		}
+	}
 	msg := &logger.Message{
 		Source:    l.Stream,
 		Timestamp: l.Created,
 		Line:      []byte(l.Log),
-		Attrs:     l.Attrs,
+		Attrs:     attrs,
 	}
 	return msg, nil
 }

daemon/logger/logger.go

@@ -68,11 +68,6 @@ func (m *Message) AsLogMessage() *backend.LogMessage {
 	return (*backend.LogMessage)(m)
 }
 
-// LogAttributes is used to hold the extra attributes available in the log message
-// Primarily used for converting the map type to string and sorting.
-// Imported here so it can be used internally with less refactoring
-type LogAttributes backend.LogAttributes
-
 // Logger is the interface for docker logging drivers.
 type Logger interface {
 	Log(*Message) error

daemon/logger/logger_test.go

@@ -1,5 +1,9 @@
 package logger
 
+import (
+	"github.com/docker/docker/api/types/backend"
+)
+
 func (m *Message) copy() *Message {
 	msg := &Message{
 		Source:    m.Source,
@@ -8,10 +12,8 @@ func (m *Message) copy() *Message {
 	}
 
 	if m.Attrs != nil {
-		msg.Attrs = make(map[string]string, len(m.Attrs))
-		for k, v := range m.Attrs {
-			msg.Attrs[k] = v
-		}
+		msg.Attrs = make([]backend.LogAttr, len(m.Attrs))
+		copy(msg.Attrs, m.Attrs)
 	}
 
 	msg.Line = append(make([]byte, 0, len(m.Line)), m.Line...)