diff --git a/balancer/balancer.go b/balancer/balancer.go index ce6493795..b179afc50 100644 --- a/balancer/balancer.go +++ b/balancer/balancer.go @@ -308,12 +308,13 @@ func (b *BalancerImpl) GetBestNode(ctx context.Context, redirectPrefixes []strin for _, prefix := range redirectPrefixes { waitGroup.Add(1) go func(prefix string) { + defer waitGroup.Done() addr, e := b.queryMistForClosestNode(ctx, playbackID, lat, lon, prefix) mu.Lock() defer mu.Unlock() if e != nil { err = e - glog.V(8).Infof("error finding origin server playbackID=%s prefix=%s error=%s", playbackID, prefix, e) + glog.V(9).Infof("error finding origin server playbackID=%s prefix=%s error=%s", playbackID, prefix, e) // If we didn't find a stream but we did find a server, keep that so we can use it to handle a 404 if addr != "" { fallbackAddr = addr @@ -322,7 +323,6 @@ func (b *BalancerImpl) GetBestNode(ctx context.Context, redirectPrefixes []strin nodeAddr = addr fullPlaybackID = prefix + "+" + playbackID } - waitGroup.Done() }(prefix) } waitGroup.Wait() diff --git a/handlers/geolocation/gelocation_test.go b/handlers/geolocation/gelocation_test.go index 3888a1f11..3f3871a4d 100644 --- a/handlers/geolocation/gelocation_test.go +++ b/handlers/geolocation/gelocation_test.go @@ -2,6 +2,7 @@ package geolocation import ( "context" + "errors" "fmt" "math/rand" "net/http" @@ -21,8 +22,10 @@ import ( ) const ( - closestNodeAddr = "someurl.com" - playbackID = "abc_XYZ-123" + closestNodeAddr = "someurl.com" + playbackID = "we91yp6cmq41niox" + CdnRedirectedPlaybackID = "cdn40y22lq7z1m8o" + UnknownPlaybackID = "unknown2aybmvI02" ) var fakeSerfMember = cluster.Member{ @@ -130,6 +133,16 @@ func mockHandlers(t *testing.T) *GeolocationHandlersCollection { AnyTimes(). Return(closestNodeAddr, fmt.Sprintf("%s+%s", prefixes[0], playbackID), nil) + mb.EXPECT(). + GetBestNode(context.Background(), prefixes[:], CdnRedirectedPlaybackID, "", "", ""). + AnyTimes(). + Return(closestNodeAddr, fmt.Sprintf("%s+%s", prefixes[0], CdnRedirectedPlaybackID), nil) + + mb.EXPECT(). + GetBestNode(context.Background(), prefixes[:], UnknownPlaybackID, "", "", ""). + AnyTimes(). + Return("", "", errors.New("")) + mc.EXPECT(). Member(map[string]string{}, "alive", closestNodeAddr). AnyTimes(). @@ -333,10 +346,9 @@ func TestNodeHostPortRedirect(t *testing.T) { func TestCdnRedirect(t *testing.T) { n := mockHandlers(t) - CdnRedirectedPlaybackId := "def_ZXY-999" - n.Config.NodeHost = "someurl.com" + n.Config.NodeHost = closestNodeAddr n.Config.CdnRedirectPrefix, _ = url.Parse("https://external-cdn.com/mist") - n.Config.CdnRedirectPlaybackIDs = []string{CdnRedirectedPlaybackId} + n.Config.CdnRedirectPlaybackIDs = []string{CdnRedirectedPlaybackID} // to be redirected to the closest node requireReq(t, fmt.Sprintf("/hls/%s/index.m3u8", playbackID)). @@ -345,38 +357,73 @@ func TestCdnRedirect(t *testing.T) { hasHeader("Location", fmt.Sprintf("http://%s/hls/%s/index.m3u8", closestNodeAddr, playbackID)) // playbackID is configured to be redirected to CDN but the path is /json_video... so redirect it to the closest node - requireReq(t, fmt.Sprintf("/json_video+%s.js", CdnRedirectedPlaybackId)). + requireReq(t, fmt.Sprintf("/json_video+%s.js", CdnRedirectedPlaybackID)). result(n). hasStatus(http.StatusTemporaryRedirect). - hasHeader("Location", fmt.Sprintf("http://%s/json_video+%s.js", closestNodeAddr, CdnRedirectedPlaybackId)) + hasHeader("Location", fmt.Sprintf("http://%s/json_video+%s.js", closestNodeAddr, CdnRedirectedPlaybackID)) + + // don't redirect if `CdnRedirectPrefix` is not configured + n.Config.CdnRedirectPrefix = nil + requireReq(t, fmt.Sprintf("/hls/%s/index.m3u8", CdnRedirectedPlaybackID)). + result(n). + hasStatus(http.StatusTemporaryRedirect). + hasHeader("Location", fmt.Sprintf("http://%s/hls/%s/index.m3u8", closestNodeAddr, CdnRedirectedPlaybackID)) +} + +func TestCdnRedirectWebRTC(t *testing.T) { + n := mockHandlers(t) + n.Config.NodeHost = closestNodeAddr + n.Config.CdnRedirectPrefix, _ = url.Parse("https://external-cdn.com/mist") + n.Config.CdnRedirectPlaybackIDs = []string{CdnRedirectedPlaybackID} // playbackID is configured to be redirected to CDN but it's /webrtc - require.Equal(t, testutil.CollectAndCount(metrics.Metrics.CDNRedirectWebRTC406), 0) - requireReq(t, fmt.Sprintf("/webrtc/%s", CdnRedirectedPlaybackId)). + requireReq(t, fmt.Sprintf("/webrtc/%s", CdnRedirectedPlaybackID)). result(n). hasStatus(http.StatusNotAcceptable) - require.Equal(t, testutil.CollectAndCount(metrics.Metrics.CDNRedirectWebRTC406), 1) + + require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectWebRTC406), float64(1)) require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectWebRTC406.WithLabelValues("unknown")), float64(0)) - require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectWebRTC406.WithLabelValues(CdnRedirectedPlaybackId)), float64(1)) + require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectWebRTC406.WithLabelValues(CdnRedirectedPlaybackID)), float64(1)) - // this playbackID is configured to be redirected to CDN - require.Equal(t, testutil.CollectAndCount(metrics.Metrics.CDNRedirectCount), 0) +} + +func TestCdnRedirectHLS(t *testing.T) { + n := mockHandlers(t) + n.Config.NodeHost = closestNodeAddr + n.Config.CdnRedirectPrefix, _ = url.Parse("https://external-cdn.com/mist") + n.Config.CdnRedirectPlaybackIDs = []string{CdnRedirectedPlaybackID} - requireReq(t, fmt.Sprintf("/hls/%s/index.m3u8", CdnRedirectedPlaybackId)). + // this playbackID is configured to be redirected to CDN + requireReq(t, fmt.Sprintf("/hls/%s/index.m3u8", CdnRedirectedPlaybackID)). result(n). hasStatus(http.StatusTemporaryRedirect). - hasHeader("Location", fmt.Sprintf("http://external-cdn.com/mist/hls/%s/index.m3u8", CdnRedirectedPlaybackId)) + hasHeader("Location", fmt.Sprintf("http://external-cdn.com/mist/hls/video+%s/index.m3u8", CdnRedirectedPlaybackID)) - require.Equal(t, testutil.CollectAndCount(metrics.Metrics.CDNRedirectCount), 1) + require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectCount), float64(1)) require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectCount.WithLabelValues("unknown")), float64(0)) - require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectCount.WithLabelValues(CdnRedirectedPlaybackId)), float64(1)) + require.Equal(t, testutil.ToFloat64(metrics.Metrics.CDNRedirectCount.WithLabelValues(CdnRedirectedPlaybackID)), float64(1)) - // don't redirect if `CdnRedirectPrefix` is not configured - n.Config.CdnRedirectPrefix = nil - requireReq(t, fmt.Sprintf("/hls/%s/index.m3u8", CdnRedirectedPlaybackId)). +} + +func TestCdnRedirectHLSUnknownPlaybackId(t *testing.T) { + n := mockHandlers(t) + n.Config.NodeHost = closestNodeAddr + n.Config.CdnRedirectPrefix, _ = url.Parse("https://external-cdn.com/mist") + n.Config.CdnRedirectPlaybackIDs = []string{CdnRedirectedPlaybackID, UnknownPlaybackID} + + // Mist doesn't know this playbackID at all + requireReq(t, fmt.Sprintf("/hls/%s/index.m3u8", UnknownPlaybackID)). result(n). - hasStatus(http.StatusTemporaryRedirect). - hasHeader("Location", fmt.Sprintf("http://%s/hls/%s/index.m3u8", closestNodeAddr, CdnRedirectedPlaybackId)) + hasStatus(http.StatusBadGateway) + + defer func() { + if err := recover(); err == nil { + panic(err) + } + }() + + // raises a panic() as there are no metrics collected + testutil.ToFloat64(metrics.Metrics.CDNRedirectCount) } diff --git a/handlers/geolocation/geolocation.go b/handlers/geolocation/geolocation.go index 0bde50bef..ed717c1b2 100644 --- a/handlers/geolocation/geolocation.go +++ b/handlers/geolocation/geolocation.go @@ -31,6 +31,9 @@ func (c *GeolocationHandlersCollection) RedirectHandler() httprouter.Handle { return func(w http.ResponseWriter, r *http.Request, params httprouter.Params) { host := r.Host pathType, prefix, playbackID, pathTmpl := parsePlaybackID(r.URL.Path) + redirectPrefixes := c.Config.RedirectPrefixes + lat := r.Header.Get("X-Latitude") + lon := r.Header.Get("X-Longitude") if c.Config.CdnRedirectPrefix != nil && (pathType == "hls" || pathType == "webrtc") { if contains(c.Config.CdnRedirectPlaybackIDs, playbackID) { @@ -43,10 +46,18 @@ func (c *GeolocationHandlersCollection) RedirectHandler() httprouter.Handle { glog.V(6).Infof("%s not supported for CDN-redirected %s", r.URL.Path, playbackID) return } + + _, fullPlaybackID, err := c.Balancer.GetBestNode(context.Background(), redirectPrefixes, playbackID, lat, lon, prefix) + if err != nil { + glog.Errorf("failed to find either origin or fallback server for playbackID=%s err=%s", playbackID, err) + w.WriteHeader(http.StatusBadGateway) + return + } + newURL, _ := url.Parse(r.URL.String()) newURL.Scheme = protocol(r) newURL.Host = c.Config.CdnRedirectPrefix.Host - newURL.Path, _ = url.JoinPath(c.Config.CdnRedirectPrefix.Path, newURL.Path) + newURL.Path, _ = url.JoinPath(c.Config.CdnRedirectPrefix.Path, fmt.Sprintf(pathTmpl, fullPlaybackID)) http.Redirect(w, r, newURL.String(), http.StatusTemporaryRedirect) metrics.Metrics.CDNRedirectCount.WithLabelValues(playbackID).Inc() glog.V(6).Infof("CDN redirect host=%s from=%s to=%s", host, r.URL, newURL) @@ -76,10 +87,6 @@ func (c *GeolocationHandlersCollection) RedirectHandler() httprouter.Handle { return } - lat := r.Header.Get("X-Latitude") - lon := r.Header.Get("X-Longitude") - - redirectPrefixes := c.Config.RedirectPrefixes bestNode, fullPlaybackID, err := c.Balancer.GetBestNode(context.Background(), redirectPrefixes, playbackID, lat, lon, prefix) if err != nil { glog.Errorf("failed to find either origin or fallback server for playbackID=%s err=%s", playbackID, err)