Include LogDog viewer links in message.

logdog/client/annotee/processor.go

 // Copyright 2015 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package annotee
 
 import (
         "bufio"
         "bytes"
         "fmt"
@@ skipping 269 matching lines @@
 
         h, err := p.getStepHandler(step, true)
         if err != nil {
                 return err
         }
 
         // Determine our injected annotations.
         injectedAnnotations := h.flushInjectedAnnotations()
 
         // Emit the "all" link if configured (at most once).
-        if s.EmitAllLink {
-                if l := buildStreamLinkAnnotation(p.o.LinkGenerator, "all", "stdio", "**/stdout", "**/stderr"); l != "" {
-                        injectedAnnotations = append(injectedAnnotations, l)
-                }
+        if lg := p.o.LinkGenerator; lg != nil && s.EmitAllLink {
+                injectedAnnotations = append(injectedAnnotations,
+                        buildAliasAnnotation("all", "stdio", lg.GetLink("**/stdout", "**/stderr")))
+
                 s.EmitAllLink = false
         }
 
         // Get our root log stream handler. As an optimization, if "step" is
         // the root step, then "h" is already the root handler, so we don't need
         // to duplicate the lookup.
         //
         // We only need the handler if we're going to emit annotations to the root
         // stream.
         var rootHandler *stepHandler
@@ skipping 45 matching lines @@
                 if !s.StripAnnotations {
                         if err := rootHandler.writeBaseStream(s, anno); err != nil {
                                 log.WithError(err).Errorf(h, "Failed to send injected annotation line to LogDog.")
                                 return err
                         }
                 }
         }
 
         // If we're stripping text, write a warning message noting that this stream
         // will not have text in it.
-        if !p.o.TeeText && s.Tee != nil && !h.textStrippedNote {
-                err := writeTextLine(s.Tee, "This build is configured to send log data exclusively to LogDog. "+
-                        "Please click the LogDog link on the build page to view this log stream.")
-                if err != nil {
-                        log.WithError(err).Errorf(h, "Failed to write text stripped notice.")
-                        return err
+        if !p.o.TeeText && s.Tee != nil {
+                if !h.textStrippedNote {
+                        err := writeTextLine(s.Tee, "This build is configured to send log data exclusively to LogDog. "+
+                                "Please use the LogDog link on the build page to view this log stream.")

[hinoka, 2017/03/27 22:21:05]

This seems like a better place to inject the viewer links.

eg. Please use the LogDog link on the build page or visit
https://luci-logdog.appspot.com/.....

[dnj, 2017/03/27 22:24:14]

We can't inject here, since the links aren't known until a stream is actually
generated for a step, but this happens the moment a step is created.

What will likely happen is:

Round 1) This message gets printed.
Round 2) New stream (STDOUT) is created, inject text, text gets printed.
Round 3) New stream (STDERR) is created, inject text, text gets printed.
etc.

+                        if err != nil {
+                                log.WithError(err).Errorf(h, "Failed to write text stripped notice.")
+                                return err
+                        }
+
+                        h.textStrippedNote = true
                 }
 
-                h.textStrippedNote = true
+                // Add links to specific log streams as they are generated.
+                injectTextStreamLines := h.flushInjectedTextStreamLines()
+                if len(injectTextStreamLines) > 0 {

[hinoka, 2017/03/27 22:21:05]

the if statement is redundant.

[dnj, 2017/03/27 22:24:13]

Done.

+                        for _, line := range injectTextStreamLines {
+                                if err := writeTextLine(s.Tee, line); err != nil {
+                                        log.WithError(err).Errorf(h, "Failed to inject text stream line: %s", line)
+                                        return err
+                                }
+                        }
+                }
         }
 
         // If this is a text line, and we're teeing text, emit this line.
         if a == "" {
                 if s.Tee != nil && p.o.TeeText {
                         if err := writeTextLine(s.Tee, line); err != nil {
                                 log.WithError(err).Errorf(h, "Failed to tee text line.")
                                 return err
                         }
                 }
@@ skipping 191 matching lines @@
         }
 }
 
 // stepHandler handles the steps associated with a specified stream.
 type stepHandler struct {
         context.Context
 
         processor *Processor
         step      *annotation.Step
 
-        client              streamclient.Client
-        injectedAnnotations []string
-        streams             map[types.StreamName]streamclient.Stream
-        finished            bool
-        allEmitted          bool
-        textStrippedNote    bool
+        client                  streamclient.Client
+        injectedAnnotations     []string
+        injectedTextStreamLines []string
+        streams                 map[types.StreamName]streamclient.Stream
+        finished                bool
+        allEmitted              bool
+        textStrippedNote        bool
 }
 
 func newStepHandler(p *Processor, step *annotation.Step) (*stepHandler, error) {
         h := stepHandler{
                 Context:   log.SetField(p.ctx, "step", step),
                 processor: p,
                 step:      step,
 
                 client:  p.o.Client,
                 streams: make(map[types.StreamName]streamclient.Stream),
@@ skipping 95 matching lines @@
 
 func (h *stepHandler) closeStreamImpl(name types.StreamName, s streamclient.Stream) {
         if err := s.Close(); err != nil {
                 log.Fields{
                         log.ErrorKey: err,
                         "stream":     name,
                 }.Errorf(h, "Failed to close step stream.")
         }
 }
 
 func (h *stepHandler) flushInjectedAnnotations() []string {

[hinoka, 2017/03/27 22:21:05]

This method and the one below looks like they could be inlined now.

[dnj, 2017/03/27 22:24:14]

The method up there is huge enough. I'd like to keep it from operating on
internal data members here.

-        if len(h.injectedAnnotations) == 0 {
+        return flushStringSlice(&h.injectedAnnotations)
+}
+
+func (h *stepHandler) flushInjectedTextStreamLines() []string {
+        return flushStringSlice(&h.injectedTextStreamLines)
+}
+
+func (h *stepHandler) maybeInjectLink(base, text string, names ...types.StreamName) {
+        if lg := h.processor.o.LinkGenerator; lg != nil {
+                link := lg.GetLink(names...)
+
+                h.injectedAnnotations = append(h.injectedAnnotations, buildAliasAnnotation(base, text, link))
+                h.injectedTextStreamLines = append(h.injectedTextStreamLines, fmt.Sprintf("LogDog Link [%s]: %s", base, link))
+        }
+}
+
+func (h *stepHandler) maybeInjectTextStreamLink(name string, stream types.StreamName) {
+        if lg := h.processor.o.LinkGenerator; lg != nil {
+        }
+}
+
+func buildAliasAnnotation(base, text, link string) string {
+        return buildAnnotation("STEP_LINK", fmt.Sprintf("%s-->%s", text, base), link)
+}
+
+func flushStringSlice(sp *[]string) []string {
+        if sp == nil {
                 return nil
         }
 
-        lines := make([]string, len(h.injectedAnnotations))
-        copy(lines, h.injectedAnnotations)
-        h.injectedAnnotations = h.injectedAnnotations[:0]
+        s := *sp
+        if len(s) == 0 {
+                return nil
+        }
+
+        lines := make([]string, len(s))
+        copy(lines, s)
+        *sp = s[:0]
 
         return lines
 }
 
-func (h *stepHandler) maybeInjectLink(base, text string, names ...types.StreamName) {
-        if l := buildStreamLinkAnnotation(h.processor.o.LinkGenerator, base, text, names...); l != "" {
-                h.injectedAnnotations = append(h.injectedAnnotations, l)
-        }
-}
-
-func buildStreamLinkAnnotation(lg LinkGenerator, base, text string, names ...types.StreamName) string {
-        if lg != nil {
-                if link := lg.GetLink(names...); link != "" {
-                        return buildAnnotation("STEP_LINK", fmt.Sprintf("%s-->%s", text, base), link)
-                }
-        }
-        return ""
-}
-
 // lineReader reads from an input stream and returns the data line-by-line.
 //
 // We don't use a Scanner because we want to be able to handle lines that may
 // exceed the buffer length. We don't use ReadBytes here because we need to
 // capture the last line in the stream, even if it doesn't end with a newline.
 type lineReader struct {
         r   *bufio.Reader
         buf bytes.Buffer
 }
 
@@ skipping 37 matching lines @@
         return "@@@" + strings.Join(append(v, params...), "@") + "@@@"
 }
 
 func isContentAnnotation(a string) bool {
         // Strip out any annotation arguments.
         if idx := strings.IndexRune(a, '@'); idx > 0 {
                 a = a[:idx]
         }
         return a == "STEP_LOG_LINE"
 }