milo: Export LogDog annotation stream, remove host

milo/appengine/logdog/build.go

 // Copyright 2016 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 logdog
 
 import (
-        "errors"
         "fmt"
         "net/http"
-        "net/url"
-        "strings"
         "time"
 
         log "github.com/luci/luci-go/common/logging"
         "github.com/luci/luci-go/common/proto/google"
         miloProto "github.com/luci/luci-go/common/proto/milo"
         "github.com/luci/luci-go/grpc/grpcutil"
         "github.com/luci/luci-go/logdog/api/logpb"
         "github.com/luci/luci-go/logdog/client/coordinator"
         "github.com/luci/luci-go/logdog/common/types"
+        "github.com/luci/luci-go/logdog/common/viewer"
         "github.com/luci/luci-go/luci_config/common/cfgtypes"
         "github.com/luci/luci-go/milo/api/resp"
         "github.com/luci/luci-go/milo/appengine/logdog/internal"
         "github.com/luci/luci-go/milo/common/miloerror"
 
         "github.com/golang/protobuf/proto"
         mc "github.com/luci/gae/service/memcache"
         "golang.org/x/net/context"
         "google.golang.org/grpc/codes"
 )
 
 const (
         // intermediateCacheLifetime is the amount of time to cache intermediate (non-
         // terminal) annotation streams. Terminal annotation streams are cached
         // indefinitely.
         intermediateCacheLifetime = 10 * time.Second
 
         // defaultLogDogHost is the default LogDog host, if one isn't specified via
         // query string.
         defaultLogDogHost = "luci-logdog.appspot.com"
 )
 
-type annotationStreamRequest struct {
-        *AnnotationStream
-
-        // host is the name of the LogDog host.
-        host string
-
-        project cfgtypes.ProjectName
-        path    types.StreamPath
+// AnnotationStream represents a LogDog annotation protobuf stream.
+type AnnotationStream struct {
+        Project cfgtypes.ProjectName
+        Path    types.StreamPath
 
         // logDogClient is the HTTP client to use for LogDog communication.
-        logDogClient *coordinator.Client
+        Client *coordinator.Client
 
         // cs is the unmarshalled annotation stream Step and associated data.
         cs internal.CachedStep
 }
 
-func (as *annotationStreamRequest) normalize() error {
-        if err := as.project.Validate(); err != nil {
-                return &miloerror.Error{
-                        Message: "Invalid project name",
-                        Code:    http.StatusBadRequest,
-                }
+// Normalize validates and normalizes the stream's parameters.
+func (as *AnnotationStream) Normalize() error {
+        if err := as.Project.Validate(); err != nil {
+                return fmt.Errorf("Invalid project name: %s", as.Project)

[hinoka, 2017/02/02 22:09:25]

This makes it a 500 instead of a 400, is that intentional?

[dnj, 2017/02/02 22:29:00]

oops - I moved that logic into "html.go" now.

         }
 
-        if err := as.path.Validate(); err != nil {
-                return &miloerror.Error{
-                        Message: fmt.Sprintf("Invalid log stream path %q: %s", as.path, err),
-                        Code:    http.StatusBadRequest,
-                }
-        }
-
-        // Get the host. We normalize it to lowercase and trim spaces since we use
-        // it as a memcache key.
-        as.host = strings.ToLower(strings.TrimSpace(as.host))
-        if as.host == "" {
-                as.host = defaultLogDogHost
-        }
-        if strings.ContainsRune(as.host, '/') {
-                return errors.New("invalid host name")
+        if err := as.Path.Validate(); err != nil {
+                return fmt.Errorf("Invalid log stream path %q: %s", as.Path, err)

[hinoka, 2017/02/02 22:09:25]

Same here

[dnj, 2017/02/02 22:29:00]

Done.

         }
 
         return nil
 }
 
-func (as *annotationStreamRequest) memcacheKey() string {
-        return fmt.Sprintf("logdog/%s/%s/%s", as.host, as.project, as.path)
-}
-
-func (as *annotationStreamRequest) load(c context.Context) error {
+// Load loads (or re-loads) the annotation stream from LogDog.
+func (as *AnnotationStream) Load(c context.Context) error {
         // Load from memcache, if possible. If an error occurs, we will proceed as if
         // no CachedStep was available.
         mcKey := as.memcacheKey()
         mcItem, err := mc.GetKey(c, mcKey)
         switch err {
         case nil:
                 if err := proto.Unmarshal(mcItem.Value(), &as.cs); err == nil {
                         return nil
                 }
 
@@ skipping 12 matching lines @@
 
         default:
                 log.Fields{
                         log.ErrorKey:  err,
                         "memcacheKey": mcKey,
                 }.Errorf(c, "Failed to load annotation protobuf memcache cached step.")
         }
 
         // Load from LogDog directly.
         log.Fields{
-                "project": as.project,
-                "path":    as.path,
-                "host":    as.host,
+                "host":    as.Client.Host,
+                "project": as.Project,
+                "path":    as.Path,
         }.Infof(c, "Making tail request to LogDog to fetch annotation stream.")
 
         var (
                 state  coordinator.LogStream
-                stream = as.logDogClient.Stream(as.project, as.path)
+                stream = as.Client.Stream(as.Project, as.Path)
         )
         le, err := stream.Tail(c, coordinator.WithState(&state), coordinator.Complete())
         switch code := grpcutil.Code(err); code {
         case codes.OK:
                 break
 
         case codes.NotFound:
                 return &miloerror.Error{
                         Message: "Stream not found",
                         Code:    http.StatusNotFound,
@@ skipping 99 matching lines @@
                 if err := mc.Set(c, mcItem); err != nil {
                         log.WithError(err).Warningf(c, "Failed to cache annotation protobuf CachedStep.")
                 }
         } else {
                 log.WithError(err).Warningf(c, "Failed to marshal annotation protobuf CachedStep.")
         }
 
         return nil
 }
 
-func (as *annotationStreamRequest) toMiloBuild(c context.Context) *resp.MiloBuild {
-        prefix, name := as.path.Split()
+// Step returns the loaded Step.
+//
+// This must be populated via Load, or else it will return nil.
+func (as *AnnotationStream) Step() *miloProto.Step { return as.cs.Step }

[hinoka, 2017/02/02 22:09:25]

Can't you just do
if as.cs.Step == nil {
  err := as.Load()
  if err != nil .....
}

As an interface, Load then access is kind of a confusing interface (esp if the
access doesn't error out)

[dnj, 2017/02/02 22:29:00]

Well, this way we can reload if we want. I suppose that if we ever needed that,
we could add an "Invalidate()" method that nils it though, so ok.

+
+func (as *AnnotationStream) memcacheKey() string {
+        return fmt.Sprintf("logdog/%s/%s/%s", as.Client.Host, as.Project, as.Path)
+}
+
+func (as *AnnotationStream) toMiloBuild(c context.Context) *resp.MiloBuild {
+        prefix, name := as.Path.Split()
 
         // Prepare a Streams object with only one stream.
         streams := Streams{
                 MainStream: &Stream{
-                        Server:     as.host,
+                        Server:     as.Client.Host,
                         Prefix:     string(prefix),
                         Path:       string(name),
                         IsDatagram: true,
                         Data:       as.cs.Step,
                         Closed:     as.cs.Finished,
                 },
         }
 
         var (
                 build resp.MiloBuild
                 ub    = logDogURLBuilder{
-                        project: as.project,
-                        host:    as.host,
+                        host:    as.Client.Host,
+                        project: as.Project,
                         prefix:  prefix,
                 }
         )
         AddLogDogToBuild(c, &ub, streams.MainStream.Data, &build)
         return &build
 }
 
 type logDogURLBuilder struct {
         host    string
         prefix  types.StreamName
         project cfgtypes.ProjectName
 }
 
 func (b *logDogURLBuilder) BuildLink(l *miloProto.Link) *resp.Link {
         switch t := l.Value.(type) {
         case *miloProto.Link_LogdogStream:
                 ls := t.LogdogStream
 
                 server := ls.Server
                 if server == "" {
                         server = b.host
                 }
 
                 prefix := types.StreamName(ls.Prefix)
                 if prefix == "" {
                         prefix = b.prefix
                 }
 
-                path := fmt.Sprintf("%s/%s", b.project, prefix.Join(types.StreamName(ls.Name)))
-                u := url.URL{
-                        Scheme: "https",
-                        Host:   server,
-                        Path:   "v/",
-                        RawQuery: url.Values{
-                                "s": []string{string(path)},
-                        }.Encode(),
-                }
-
+                u := viewer.GetURL(server, b.project, prefix.Join(types.StreamName(ls.Name)))
                 link := resp.Link{
                         Label: l.Label,
-                        URL:   u.String(),
+                        URL:   u,
                 }
                 if link.Label == "" {
                         link.Label = ls.Name
                 }
                 return &link
 
         case *miloProto.Link_Url:
                 link := resp.Link{
                         Label: l.Label,
                         URL:   t.Url,
                 }
                 if link.Label == "" {
                         link.Label = "unnamed"
                 }
                 return &link
 
         default:
                 // Don't know how to render.
                 return nil
         }
 }