Repository: google/certificate-transparency-go
Pull request #606 (2019-09-17 14:57:30)

Adding metric tracking time between Log-list updates

codecov[bot]: # [Codecov](https://codecov.io/gh/google/certificate-transparency-go/pull/606?src=pr&el=h1) Report
> Merging [#606](https://codecov.io/gh/google/certificate-transparency-go/pull/606?src=pr&el=desc) into [master](https://codecov.io/gh/google/certificate-transparency-go/commit/496c2e82955b5ae3cde7f00c5a2a902bc24366a0?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `92.3%`.

[![Impacted file tree graph](https://codecov.io/gh/google/certificate-transparency-go/pull/606/graphs/tree.svg?width=650&token=kYPnjCLppA&height=150&src=pr)](https://codecov.io/gh/google/certificate-transparency-go/pull/606?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #606      +/-   ##
==========================================
+ Coverage   71.83%   71.85%   +0.02%     
==========================================
  Files          90       90              
  Lines        9774     9782       +8     
==========================================
+ Hits         7021     7029       +8     
  Misses       2260     2260              
  Partials      493      493
```


| [Impacted Files](https://codecov.io/gh/google/certificate-transparency-go/pull/606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [submission/distributor.go](https://codecov.io/gh/google/certificate-transparency-go/pull/606/diff?src=pr&el=tree#diff-c3VibWlzc2lvbi9kaXN0cmlidXRvci5nbw==) | `82.38% <ø> (ø)` | :arrow_up: |
| [submission/proxy\_server.go](https://codecov.io/gh/google/certificate-transparency-go/pull/606/diff?src=pr&el=tree#diff-c3VibWlzc2lvbi9wcm94eV9zZXJ2ZXIuZ28=) | `0% <0%> (ø)` | :arrow_up: |
| [submission/loglist\_refresher.go](https://codecov.io/gh/google/certificate-transparency-go/pull/606/diff?src=pr&el=tree#diff-c3VibWlzc2lvbi9sb2dsaXN0X3JlZnJlc2hlci5nbw==) | `71.42% <100%> (-1.91%)` | :arrow_down: |
| [submission/loglist\_manager.go](https://codecov.io/gh/google/certificate-transparency-go/pull/606/diff?src=pr&el=tree#diff-c3VibWlzc2lvbi9sb2dsaXN0X21hbmFnZXIuZ28=) | `82.97% <100%> (+4.6%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/certificate-transparency-go/pull/606?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/certificate-transparency-go/pull/606?src=pr&el=footer). Last update [496c2e8...ff1d24e](https://codecov.io/gh/google/certificate-transparency-go/pull/606?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Mercurrent: PTAL
Mercurrent: Re-assigning to Paul. Rob is ill.
Mercurrent: PTAL
Mercurrent: PTAL

Patch:
diff --git a/submission/loglist_manager.go b/submission/loglist_manager.go
index 64d18ddc..68b14df5 100644
--- a/submission/loglist_manager.go
+++ b/submission/loglist_manager.go
@@ -21,8 +21,19 @@ import (
 	"time"
 
 	"github.com/google/certificate-transparency-go/schedule"
+	"github.com/google/trillian/monitoring"
 )
 
+var (
+	logRefOnce         sync.Once
+	logListLastRefresh monitoring.Gauge // Unix time
+)
+
+// logRefInitMetrics initializes all the exported metrics.
+func logRefInitMetrics(ctx context.Context, mf monitoring.MetricFactory) {
+	logListLastRefresh = mf.NewGauge("log_list_last_refresh", "Timestamp for last successful Log-list refresh")
+}
+
 // LogListManager runs loglist updates and keeps two latest versions of Log
 // list.
 type LogListManager struct {
@@ -35,14 +46,20 @@ type LogListManager struct {
 	latestLL   *LogListData
 	previousLL *LogListData
 	mu         sync.Mutex // guards latestLL and previousLL
+
+	mtf monitoring.MetricFactory
 }
 
 // NewLogListManager creates and inits a LogListManager instance.
-func NewLogListManager(llr LogListRefresher) *LogListManager {
+func NewLogListManager(llr LogListRefresher, mf monitoring.MetricFactory) *LogListManager {
+	if mf == nil {
+		mf = monitoring.InertMetricFactory{}
+	}
 	return &LogListManager{
 		Errors:    make(chan error, 1),
 		LLUpdates: make(chan LogListData, 1),
 		llr:       llr,
+		mtf:       mf,
 	}
 }
 
@@ -52,6 +69,7 @@ func NewLogListManager(llr LogListRefresher) *LogListManager {
 func (llm *LogListManager) Run(ctx context.Context, llRefresh time.Duration) {
 	llm.llRefreshInterval = llRefresh
 	go schedule.Every(ctx, llm.llRefreshInterval, llm.refreshLogListAndNotify)
+	logRefOnce.Do(func() { logRefInitMetrics(ctx, llm.mtf) })
 }
 
 // refreshLogListAndNotify runs single Log-list refresh and propagates data and
@@ -80,6 +98,7 @@ func (llm *LogListManager) RefreshLogList(ctx context.Context) (*LogListData, er
 	if err != nil {
 		return nil, err
 	}
+	logListLastRefresh.Set(float64(time.Now().Unix()))
 	if ll == nil {
 		// No updates
 		return nil, nil
diff --git a/submission/loglist_refresher.go b/submission/loglist_refresher.go
index 36179883..d199474b 100644
--- a/submission/loglist_refresher.go
+++ b/submission/loglist_refresher.go
@@ -66,10 +66,7 @@ func NewCustomLogListRefresher(client *http.Client, llPath string) LogListRefres
 // NewLogListRefresher creates and inits a LogListRefresherImpl instance using
 // default http.Client
 func NewLogListRefresher(llPath string) LogListRefresher {
-	return &logListRefresherImpl{
-		path:   llPath,
-		client: &http.Client{Timeout: httpClientTimeout},
-	}
+	return NewCustomLogListRefresher(&http.Client{Timeout: httpClientTimeout}, llPath)
 }
 
 // Refresh fetches the log list and returns its source, formed LogList and
diff --git a/submission/proxy_server.go b/submission/proxy_server.go
index e2cde5d0..da8acfc3 100644
--- a/submission/proxy_server.go
+++ b/submission/proxy_server.go
@@ -39,7 +39,7 @@ type ProxyServer struct {
 // NewProxyServer creates ProxyServer instance. Call Run() to init.
 func NewProxyServer(logListPath string, dBuilder DistributorBuilder, reqTimeout time.Duration, mf monitoring.MetricFactory) *ProxyServer {
 	s := &ProxyServer{addTimeout: reqTimeout}
-	s.p = NewProxy(NewLogListManager(NewLogListRefresher(logListPath)), dBuilder, mf)
+	s.p = NewProxy(NewLogListManager(NewLogListRefresher(logListPath), mf), dBuilder, mf)
 	return s
 }
 


Test patch:
diff --git a/submission/loglist_manager_test.go b/submission/loglist_manager_test.go
index 5fa15557..3eff46eb 100644
--- a/submission/loglist_manager_test.go
+++ b/submission/loglist_manager_test.go
@@ -23,10 +23,11 @@ import (
 	"time"
 
 	"github.com/google/certificate-transparency-go/testdata"
+	"github.com/google/trillian/monitoring"
 )
 
 func TestNoLLRefresher(t *testing.T) {
-	llm := NewLogListManager(nil)
+	llm := NewLogListManager(nil, nil)
 	_, err := llm.RefreshLogList(context.Background())
 	if err == nil {
 		t.Errorf("llm.RefreshLogList() on nil LogListRefresher expected to get error, got none")
@@ -34,7 +35,7 @@ func TestNoLLRefresher(t *testing.T) {
 }
 
 func TestNoLLRefresherAfterRun(t *testing.T) {
-	llm := NewLogListManager(nil)
+	llm := NewLogListManager(nil, nil)
 	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
 	defer cancel()
 
@@ -55,7 +56,7 @@ func TestFirstRefresh(t *testing.T) {
 	defer os.Remove(f)
 
 	llr := NewLogListRefresher(f)
-	llm := NewLogListManager(llr)
+	llm := NewLogListManager(llr, nil)
 	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
 	defer cancel()
 
@@ -78,7 +79,7 @@ func TestSecondRefresh(t *testing.T) {
 	defer os.Remove(f)
 
 	llr := NewLogListRefresher(f)
-	llm := NewLogListManager(llr)
+	llm := NewLogListManager(llr, monitoring.InertMetricFactory{})
 	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
 	defer cancel()
 	halfCtx, halfCancel := context.WithTimeout(context.Background(), time.Second)
diff --git a/submission/proxy_test.go b/submission/proxy_test.go
index 5471c594..e3f76014 100644
--- a/submission/proxy_test.go
+++ b/submission/proxy_test.go
@@ -48,7 +48,7 @@ func (llr stubLogListRefresher) Source() string {
 }
 
 func stubLogListManager() *LogListManager {
-	return NewLogListManager(stubLogListRefresher{})
+	return NewLogListManager(stubLogListRefresher{}, nil)
 }
 
 var imf monitoring.InertMetricFactory
@@ -100,7 +100,7 @@ func TestProxyInitState(t *testing.T) {
 	defer os.Remove(f)
 
 	llr := NewLogListRefresher(f)
-	p := NewProxy(NewLogListManager(llr), GetDistributorBuilder(ChromeCTPolicy, buildStubNoRootsLogClient, imf), imf)
+	p := NewProxy(NewLogListManager(llr, nil), GetDistributorBuilder(ChromeCTPolicy, buildStubNoRootsLogClient, imf), imf)
 	p.Run(context.Background(), 100*time.Millisecond, time.Hour)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)