diff --git a/src/github.com/getlantern/flashlight/analytics/analytics.go b/src/github.com/getlantern/flashlight/analytics/analytics.go index 2c2b4d0b4c..e4c24a46cf 100644 --- a/src/github.com/getlantern/flashlight/analytics/analytics.go +++ b/src/github.com/getlantern/flashlight/analytics/analytics.go @@ -14,7 +14,6 @@ import ( "github.com/getlantern/eventual" "github.com/getlantern/flashlight/client" "github.com/getlantern/flashlight/geolookup" - "github.com/getlantern/flashlight/logging" "github.com/getlantern/flashlight/util" "github.com/kardianos/osext" @@ -33,51 +32,40 @@ var ( maxWaitForIP = math.MaxInt32 * time.Second - // We get the user agent to use from live data on the proxy, but don't wait - // for it forever! - maxWaitForUserAgent = 30 * time.Second - - // This allows us to report a real user agent from clients we see on the - // proxy. - userAgent = eventual.NewValue() - // Hash of the executable hash = getExecutableHash() ) // Start starts the GA session with the given data. func Start(deviceID, version string) func() { - return start(deviceID, version, geolookup.GetIP, maxWaitForUserAgent, trackSession) + return start(deviceID, version, geolookup.GetIP, trackSession) } // start starts the GA session with the given data. -func start(deviceID, version string, ipFunc func(time.Duration) string, uaWait time.Duration, +func start(deviceID, version string, ipFunc func(time.Duration) string, transport func(string, eventual.Getter)) func() { var addr atomic.Value go func() { - logging.AddUserAgentListener(func(agent string) { - userAgent.Set(agent) - }) ip := ipFunc(maxWaitForIP) if ip == "" { log.Errorf("No IP found within %v", maxWaitForIP) } addr.Store(ip) log.Debugf("Starting analytics session with ip %v", ip) - startSession(ip, version, client.Addr, deviceID, transport, uaWait) + startSession(ip, version, client.Addr, deviceID, transport) }() stop := func() { if addr.Load() != nil { ip := addr.Load().(string) log.Debugf("Ending analytics session with ip %v", ip) - endSession(ip, version, client.Addr, deviceID, transport, uaWait) + endSession(ip, version, client.Addr, deviceID, transport) } } return stop } -func sessionVals(ip, version, clientID, sc string, uaWait time.Duration) string { +func sessionVals(ip, version, clientID, sc string) string { vals := make(url.Values, 0) vals.Add("v", "1") @@ -99,24 +87,15 @@ func sessionVals(ip, version, clientID, sc string, uaWait time.Duration) string // Custom dimension for the Lantern version vals.Add("cd1", version) - // Custom dimension for the hash of the executable - vals.Add("cd2", hash) - - // This sets the user agent to a real user agent the user is using. We - // wait 30 seconds for some traffic to come through. - ua, found := userAgent.Get(uaWait) - if found { - vals.Add("ua", ua.(string)) - } + // Custom dimension for the hash of the executable. We combine the version + // to make it easier to interpret in GA. + vals.Add("cd2", version+"-"+hash) // This forces the recording of the session duration. It must be either // "start" or "end". See: // https://developers.google.com/analytics/devguides/collection/protocol/v1/parameters vals.Add("sc", sc) - // Make this a non-interaction hit that bypasses things like bounce rate. See: - // https://developers.google.com/analytics/devguides/collection/protocol/v1/parameters#ni - vals.Add("ni", "1") return vals.Encode() } @@ -142,14 +121,14 @@ func getExecutableHash() string { } func endSession(ip string, version string, proxyAddrFN eventual.Getter, - clientID string, transport func(string, eventual.Getter), uaWait time.Duration) { - args := sessionVals(ip, version, clientID, "end", uaWait) + clientID string, transport func(string, eventual.Getter)) { + args := sessionVals(ip, version, clientID, "end") transport(args, proxyAddrFN) } func startSession(ip string, version string, proxyAddrFN eventual.Getter, - clientID string, transport func(string, eventual.Getter), uaWait time.Duration) { - args := sessionVals(ip, version, clientID, "start", uaWait) + clientID string, transport func(string, eventual.Getter)) { + args := sessionVals(ip, version, clientID, "start") transport(args, proxyAddrFN) } diff --git a/src/github.com/getlantern/flashlight/analytics/analytics_test.go b/src/github.com/getlantern/flashlight/analytics/analytics_test.go index 7685cabdae..de48b707b7 100644 --- a/src/github.com/getlantern/flashlight/analytics/analytics_test.go +++ b/src/github.com/getlantern/flashlight/analytics/analytics_test.go @@ -1,6 +1,8 @@ package analytics import ( + "io/ioutil" + "net/http" "strings" "testing" "time" @@ -16,7 +18,7 @@ func TestAnalytics(t *testing.T) { params := eventual.NewValue() start("1", "2.2.0", func(time.Duration) string { return "127.0.0.1" - }, 1*time.Millisecond, func(args string, addr eventual.Getter) { + }, func(args string, addr eventual.Getter) { logger.Debugf("Got args %v", args) params.Set(args) }) @@ -27,4 +29,16 @@ func TestAnalytics(t *testing.T) { argString := args.(string) assert.True(t, strings.Contains(argString, "pageview")) assert.True(t, strings.Contains(argString, "127.0.0.1")) + + // Now actually hit the GA debug server to validate the hit. + url := "https://www.google-analytics.com/debug/collect?" + argString + resp, err := http.Get(url) + assert.Nil(t, err, "Should be nil") + + defer resp.Body.Close() + + body, err := ioutil.ReadAll(resp.Body) + assert.Nil(t, err, "Should be nil") + + assert.True(t, strings.Contains(string(body), "\"valid\": true"), "Should be a valid hit") } diff --git a/src/github.com/getlantern/flashlight/logging/logging_test.go b/src/github.com/getlantern/flashlight/logging/logging_test.go index 2a828b7e5e..a9d0b6e41e 100644 --- a/src/github.com/getlantern/flashlight/logging/logging_test.go +++ b/src/github.com/getlantern/flashlight/logging/logging_test.go @@ -9,7 +9,6 @@ import ( "testing" "time" - "github.com/getlantern/eventual" "github.com/getlantern/go-loggly" "github.com/getlantern/golog" "github.com/stretchr/testify/assert" @@ -18,28 +17,19 @@ import ( // Test to make sure user agent registration, listening, etc is all working. func TestUserAgent(t *testing.T) { agent := "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.86 Safari/537.36" - userAgent := eventual.NewValue() - - listener := func(a string) { - userAgent.Set(a) - } // Do an initial register just to test the duplicate agent paths. RegisterUserAgent(agent) - AddUserAgentListener(listener) - go func() { - time.Sleep(200 * time.Millisecond) RegisterUserAgent(agent) }() - received, _ := userAgent.Get(4 * time.Second) - assert.Equal(t, agent, received.(string), "Unexpected agent!") + time.Sleep(200 * time.Millisecond) agents := getSessionUserAgents() - assert.True(t, strings.Contains(agents, "AppleWebKit"), "Expected agent not there!") + assert.True(t, strings.Contains(agents, "AppleWebKit"), "Expected agent not in "+agents) } type BadWriter struct{} diff --git a/src/github.com/getlantern/flashlight/logging/useragents.go b/src/github.com/getlantern/flashlight/logging/useragents.go index b674a6fcda..be095b14ab 100644 --- a/src/github.com/getlantern/flashlight/logging/useragents.go +++ b/src/github.com/getlantern/flashlight/logging/useragents.go @@ -11,16 +11,8 @@ var ( userAgents = make(map[string]int) agentsMutex = &sync.Mutex{} reg = regexp.MustCompile("^Go.*package http$") - listeners = make([]func(string), 0) ) -// AddUserAgentListener registers a listener for user agents. -func AddUserAgentListener(listener func(string)) { - agentsMutex.Lock() - defer agentsMutex.Unlock() - listeners = append(listeners, listener) -} - // RegisterUserAgent tries to find the User-Agent in the HTTP request // and keep track of the applications using Lantern during this session func RegisterUserAgent(agent string) { @@ -34,11 +26,6 @@ func RegisterUserAgent(agent string) { userAgents[agent] = n + 1 } else { userAgents[agent] = 1 - - // Only notify listeners when we have a new agent. - for _, f := range listeners { - f(agent) - } } } }()