milo: Export LogDog annotation stream, remove host

milo/appengine/logdog/build.go

Index: milo/appengine/logdog/build.go
diff --git a/milo/appengine/logdog/build.go b/milo/appengine/logdog/build.go
index fba82c954f3828062260cad5360bda4304e8304d..877f5a7f470da494b2ff0ec4e78f8c1c829386e4 100644
--- a/milo/appengine/logdog/build.go
+++ b/milo/appengine/logdog/build.go
@@ -5,11 +5,8 @@
 package logdog
 
 import (
-	"errors"
 	"fmt"
 	"net/http"
-	"net/url"
-	"strings"
 	"time"
 
 	log "github.com/luci/luci-go/common/logging"
@@ -19,6 +16,7 @@ import (
 	"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"
@@ -41,55 +39,33 @@ const (
 	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()
@@ -122,14 +98,14 @@ func (as *annotationStreamRequest) load(c context.Context) error {
 
 	// 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 {
@@ -249,13 +225,22 @@ func (as *annotationStreamRequest) load(c context.Context) error {
 	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,
@@ -267,8 +252,8 @@ func (as *annotationStreamRequest) toMiloBuild(c context.Context) *resp.MiloBuil
 	var (
 		build resp.MiloBuild
 		ub    = logDogURLBuilder{
-			project: as.project,
-			host:    as.host,
+			host:    as.Client.Host,
+			project: as.Project,
 			prefix:  prefix,
 		}
 	)
@@ -297,19 +282,10 @@ func (b *logDogURLBuilder) BuildLink(l *miloProto.Link) *resp.Link {
 			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