New metric: CPU usage #5545

kvch · 2017-11-08T19:08:33Z

Add both percentage and normalized percentage of CPU.
Move Round function to common.

houndci-bot · 2017-11-08T19:08:41Z

libbeat/cmd/instance/metrics.go

+
+	dTime := sample.time.Sub(lastSample.time)
+	dMilli := dTime / time.Millisecond
+	dCpu := int64(sample.procTimes.Total - lastSample.procTimes.Total)


var dCpu should be dCPU

houndci-bot · 2017-11-08T19:08:41Z

libbeat/cmd/instance/metrics.go

+	monitoring.ReportFloat(V, "usage.normalized", normalizedCpu)
+}
+
+func getCpuUsage() (float64, float64) {


func getCpuUsage should be getCPUUsage

houndci-bot · 2017-11-08T19:08:41Z

libbeat/cmd/instance/metrics.go

+	V.OnRegistryStart()
+	defer V.OnRegistryFinished()
+
+	cpuUsage, normalizedCpu := getCpuUsage()


var normalizedCpu should be normalizedCPU

houndci-bot · 2017-11-08T19:08:41Z

libbeat/cmd/instance/metrics.go

@@ -36,3 +56,38 @@ func reportInfo(_ monitoring.Mode, V monitoring.Visitor) {
 	uptime := int64(delta.Seconds())*1000 + int64(delta.Nanoseconds()/1000000)
 	monitoring.ReportInt(V, "uptime.ms", uptime)
 }
+
+func reportCpu(_ monitoring.Mode, V monitoring.Visitor) {


func reportCpu should be reportCPU

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

+
+	dTime := sample.time.Sub(lastSample.time)
+	dMilli := dTime / time.Millisecond
+	dCpu := int64(sample.procTimes.Total - lastSample.procTimes.Total)


var dCpu should be dCPU

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

+	monitoring.ReportFloat(V, "usage.normalized", normalizedCpu)
+}
+
+func getCpuUsage() (float64, float64) {


func getCpuUsage should be getCPUUsage

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

+	V.OnRegistryStart()
+	defer V.OnRegistryFinished()
+
+	cpuUsage, normalizedCpu := getCpuUsage()


var normalizedCpu should be normalizedCPU

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

@@ -36,3 +56,38 @@ func reportInfo(_ monitoring.Mode, V monitoring.Visitor) {
 	uptime := int64(delta.Seconds())*1000 + int64(delta.Nanoseconds()/1000000)
 	monitoring.ReportInt(V, "uptime.ms", uptime)
 }
+
+func reportCpu(_ monitoring.Mode, V monitoring.Visitor) {


func reportCpu should be reportCPU

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

+
+	dTime := sample.time.Sub(lastSample.time)
+	dMilli := dTime / time.Millisecond
+	dCpu := int64(sample.procTimes.Total - lastSample.procTimes.Total)


var dCpu should be dCPU

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

+	monitoring.ReportFloat(V, "usage.normalized", normalizedCpu)
+}
+
+func getCpuUsage() (float64, float64) {


func getCpuUsage should be getCPUUsage

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

+	V.OnRegistryStart()
+	defer V.OnRegistryFinished()
+
+	cpuUsage, normalizedCpu := getCpuUsage()


var normalizedCpu should be normalizedCPU

houndci-bot · 2017-11-08T19:08:42Z

libbeat/cmd/instance/metrics.go

@@ -36,3 +56,38 @@ func reportInfo(_ monitoring.Mode, V monitoring.Visitor) {
 	uptime := int64(delta.Seconds())*1000 + int64(delta.Nanoseconds()/1000000)
 	monitoring.ReportInt(V, "uptime.ms", uptime)
 }
+
+func reportCpu(_ monitoring.Mode, V monitoring.Visitor) {


func reportCpu should be reportCPU

ph

Changes concerning the double struct and the value of the cpu usage.

ph · 2017-11-08T20:05:37Z

libbeat/cmd/instance/metrics.go

+	}{
+		time.Now(),
+		sigar.ProcTime{},
+	}


We are using the same anonymized struct in two different places in the code We can make a concrete struct maybe called CpuSample and reuse it or we could also encapsulate the creation of the sample into his own method and return the sample or error.

ph · 2017-11-08T20:15:59Z

libbeat/cmd/instance/metrics.go

+	dCPU := int64(sample.procTimes.Total - lastSample.procTimes.Total)
+
+	usage := float64(dCPU) / float64(dMilli)
+	normalized := usage / float64(numCores)


I think I might be missing some context here concerning the nature of the recording.
This will give us a rate of cpu usage?

Yes, it gives the total CPU usage of a beat between the last two samples.

I used the method from metricbeat/modules/system/process/helper. I thought about extracting the whole Process and ProcStats and moving it libbeat. However, it seemed unnecessary to move such a big part of metricbeat into libbeat for only this minor calculation. But I am open to reconsider moving it.

All fine then! thanks @kvch !

ph · 2017-11-08T20:23:54Z

libbeat/common/math.go

+	}
+	newVal = round / pow
+	return
+}


🐼 I wish it was in the standard library, I think we should make this method takes an argument for the precision and create a method in the metrics that use the maxDecimalPlaces, since it's in the common file I don't think we should make arbitrary decision on the precision.

But It can be up to discussion

I implemented it

tsg · 2017-11-09T09:21:21Z

metricbeat/module/system/util.go

 	"runtime"

 	sigar "github.com/elastic/gosigar"
+	"github.com/elastic/libbeat/common"


I assume this should be github.com/elastic/beats/libbeat/common

tsg · 2017-11-09T09:25:12Z

metricbeat/module/system/util.go

-		OneMinute:     Round(m.sample.One),
-		FiveMinute:    Round(m.sample.Five),
-		FifteenMinute: Round(m.sample.Fifteen),
+		OneMinute:     common.Round(m.sample.One),


These need updating to the new Round() signature.

tsg · 2017-11-09T09:27:52Z

The new metrics (beat.cpu.usage, beat.cpu.usage.normalized) should be added to the list here.

Btw, I think needs to be done also for the beat.info.uptime.ms metric from the other PR.

tsg · 2017-11-09T09:31:01Z

@ruflin I wonder if we should replace beat with agent as a namespace for these before they start being used. I'm thinking in the context of the common schema, ofc.

kvch · 2017-11-09T11:58:02Z

I added beat.cpu.* to gauges list. I also added beat.info.uptime.ms to the list in a separate commit. Should I cherry-pick it to a new branch and open a new PR?

ph

Code LGTM, I think we just need the input of @ruflin concerning the common schema.

ruflin · 2017-11-09T23:41:40Z

libbeat/cmd/instance/metrics.go

+
+	dTime := sample.time.Sub(lastSample.time)
+	dMilli := dTime / time.Millisecond
+	dCPU := int64(sample.procTimes.Total - lastSample.procTimes.Total)


If we would only report sample.procTimes.Total with a timestamp + # of cores, we could leave the calculation of 1, 5 and 15 minutes average to Kibana? Like this it would be up to KB to decide which averages it wants to calculate. Perhaps I miss something?

I like your idea. @tsullivan WDYT?

Not sure I'm following... is this for system load average?

cc @pickypg

Right now it returns the number of CPU usage between the last two sampling times. Let's say that the sampling time is 1 minute. It would return the following metrics in case of constant usage:

beat.cpu.usage=0.1 // at 12:00 beat.cpu.usage=0.1 // at 12:01 beat.cpu.usage=0.1 // at 12:02

@ruflin's idea is to return a timestamp and the total CPU usage since the Beat has been started. It would look like this:

timestamp=12:00 beat.cpu.usage=0.1 timestamp=12:01 beat.cpu.usage=0.2 timestamp=12:02 beat.cpu.usage=0.3

@ruflin please, correct me if I misunderstood your idea.

Exactly. So on the UI side there would be fully flexibility if it should be 1,5,15 or 1,10,30 minute average and we report only one value each time.

A potential problem came to my mind. It might be confusing to users to see that the internal metrics are reported differently from metricbeat's format. For the sake of consistency, I would stick with the current solution. (Even if it's not as flexible as your proposal is.)

So my only concern with this approach is that it's different from what we're currently doing.

In the short term, can we have both? The other parts of the stack also report it in the not-rolled-up format, so we it may be confusing to present a different, albeit better approach to showing the utilization. However, longer term, I would love to see us move toward the more flexible approach, which is only possible if we already have the data (and it seems cheap to have both).

Having both SGTM

ruflin · 2017-11-09T23:42:21Z

libbeat/logp/metrics.go

@@ -71,6 +71,9 @@ var gauges = map[string]bool{
 	"beat.memstats.memory_total":     true,
 	"beat.memstats.memory_alloc":     true,
 	"beat.memstats.gc_next":          true,
+	"beat.cpu.usage":                 true,


@tsg At one point we should review these metrics to align them with metricbeat so it would be system.cpu.* .

ruflin · 2017-11-09T23:43:11Z

@tsg Yes, we should review the naming of all metrics. So my comment inline.

andrewkroh · 2017-11-13T19:59:06Z

libbeat/cmd/instance/metrics.go

+	numCores   = runtime.NumCPU()
+	lastSample = cpuSample{
+		time:      time.Now(),
+		procTimes: sigar.ProcTime{},


Personally I would omit this line because it doesn't do any more initialization than the zero value you get by default. (Same for line 77.)

andrewkroh · 2017-11-13T20:07:30Z

libbeat/cmd/instance/metrics.go

 )

 func init() {
+	pid := os.Getpid()
+	err := lastSample.procTimes.Get(pid)


Get() is not implemented for all platforms so by adding this you will be limiting the platforms for which Beats can be compiled. This should probably be stubbed in gosigar, but that is currently not the case so you'll need to only use it on the platforms where it's available (linux, windows, openbsd, darwin).

andrewkroh · 2017-11-13T20:12:43Z

libbeat/cmd/instance/metrics.go

+	pid := os.Getpid()
+	err := lastSample.procTimes.Get(pid)
+	if err != nil {
+		panic(err)


I think you should handle the error rather than panic'ing. I would reserve the use of panics to cases that are completely unrecoverable or are programmer errors that are covered by unit tests (like hitting the default case in a switch statement that is supposed to cover all enum values). Panics on init can be really hard to debug especially on Windows.

andrewkroh · 2017-11-13T20:30:29Z

libbeat/cmd/instance/metrics.go

+	monitoring.ReportFloat(V, "usage.normalized", normalizedCPU)
+}
+
+func getCPUUsage() (float64, float64) {


Suggest getProcessCPUUsage to make it clear that it's CPU usage for this process only vs overall CPU usage. It would be nice to have a godoc statement explaining what the return values are since there are two of them (and what their ranges are like [0, 1], [0, 100], [0, 100 * num_cpus], etc.).

andrewkroh · 2017-11-13T20:31:42Z

libbeat/cmd/instance/metrics.go

+
+	dTime := sample.time.Sub(lastSample.time)
+	dMilli := dTime / time.Millisecond
+	dCPU := int64(sample.procTimes.Total - lastSample.procTimes.Total)


Can this value be negative? (like if the counter rolls over) If we just send the raw counter value this problem goes to someone else :).

andrewkroh · 2017-11-13T20:33:50Z

libbeat/cmd/instance/metrics.go

+		procTimes: sigar.ProcTime{},
+	}
+
+	if err := sample.procTimes.Get(pid); err != nil {


I'd prefer to return the error and have the caller handle it. The caller might want to log it so we have some way to debug things when a user tells us they are only seeing 0 for CPU usage.

andrewkroh · 2017-11-13T20:43:27Z

libbeat/cmd/instance/metrics.go

 	metrics := monitoring.Default.NewRegistry("beat")

 	monitoring.NewFunc(metrics, "memstats", reportMemStats, monitoring.Report)
+	monitoring.NewFunc(metrics, "cpu", reportCPU, monitoring.Report)


If the Beat is running on a platform where we have no support for getting process CPU data maybe we should not register this metric?

@tsullivan I assume in the UI it would be best to tell the user (like n/a) that we don't have CPU usage data rather than showing 0. Would not sending the metric be the way to go?

houndci-bot · 2017-11-14T16:46:41Z

libbeat/cmd/instance/metrics.go

@@ -1,16 +1,40 @@
+// +build darwin freebsd linux openbsd windows


package comment should be of the form "Package instance ..."

kvch · 2017-11-15T17:41:57Z

I opened a PR in gosigar, so crosscompile will be fixed: elastic/gosigar#83

houndci-bot · 2017-11-28T16:19:45Z

libbeat/metrics/system/system.go

+	sample *sigar.LoadAverage
+}
+
+type LoadAverages struct {


exported type LoadAverages should have comment or be unexported

houndci-bot · 2017-11-28T16:19:46Z

libbeat/metrics/system/system.go

+	return &LoadMetrics{load}, nil
+}
+
+type LoadMetrics struct {


exported type LoadMetrics should have comment or be unexported

houndci-bot · 2017-11-28T16:19:46Z

libbeat/metrics/system/system.go

+
+// CPU Core Monitor
+
+// CPUCoreMonitor is used to monitor the usage of individual CPU cores.


comment on exported type CPUCoreMetrics should be of the form "CPUCoreMetrics ..." (with optional leading article)

houndci-bot · 2017-11-28T16:19:46Z

libbeat/metrics/system/system.go

+	}
+}
+
+func (m *CPUMetrics) Ticks() CPUTicks {


exported method CPUMetrics.Ticks should have comment or be unexported

houndci-bot · 2017-11-28T16:19:46Z

libbeat/metrics/system/system.go

+	Steal   uint64
+}
+
+type CPUMetrics struct {


exported type CPUMetrics should have comment or be unexported

houndci-bot · 2017-11-28T16:19:46Z

libbeat/metrics/process/process.go

@@ -292,7 +293,7 @@ func (procStats *ProcStats) getProcessEvent(process *Process) common.MapStr {
 // time multiplied by the number of cores as the total amount of CPU time
 // available between samples. This could result in incorrect percentages if the
 // wall-clock is adjusted (prior to Go 1.9) or the machine is suspended.
-func GetProcCpuPercentage(s0, s1 *Process) (normalizedPct, pct float64) {
+func GetProcCpuPercentage(s0, s1 *Process) (normalizedPct, pct, totalPct float64) {


func GetProcCpuPercentage should be GetProcCPUPercentage

houndci-bot · 2017-11-28T16:19:47Z

libbeat/metrics/process/process.go

 	cpuTotalPct     float64
 	cpuTotalPctNorm float64
 }

-type ProcStats struct {
+type Stats struct {


exported type Stats should have comment or be unexported

houndci-bot · 2017-11-28T16:19:47Z

libbeat/metrics/process/config.go

@@ -0,0 +1,9 @@
+package process
+
+// includeTopConfig is the configuration for the "top N processes


comment on exported type IncludeTopConfig should be of the form "IncludeTopConfig ..." (with optional leading article)

houndci-bot · 2017-11-28T16:19:47Z

libbeat/cmd/instance/metrics.go

+		return 0.0, 0.0, 0.0, fmt.Errorf("error converting value of normalized CPU usage")
+	}
+
+	totalCpuUsage, ok := iTotalCpuUsage.(float64)


var totalCpuUsage should be totalCPUUsage

houndci-bot · 2017-11-28T16:19:47Z

libbeat/cmd/instance/metrics.go

+		return 0.0, 0.0, 0.0, fmt.Errorf("error getting normalized CPU percentage: %v", err)
+	}
+
+	iTotalCpuUsage, err := beatState.GetValue("cpu.total.total_since_start")


var iTotalCpuUsage should be iTotalCPUUsage

houndci-bot · 2017-11-28T16:40:07Z

libbeat/metrics/system/system.go

+	Total   float64
+}
+
+// CPUPercentages stores all CPU values in number of tick collected by a Beat.


comment on exported type CPUTicks should be of the form "CPUTicks ..." (with optional leading article)

houndci-bot · 2017-11-28T16:40:07Z

libbeat/metrics/process/process.go

@@ -441,12 +450,12 @@ func isProcessInSlice(processes []Process, proc *Process) bool {

 // isWhitelistedEnvVar returns true if the given variable name is a match for
 // the whitelist. If the whitelist is empty it returns false.
-func (p ProcStats) isWhitelistedEnvVar(varName string) bool {
-	if len(p.envRegexps) == 0 {
+func (s Stats) isWhitelistedEnvVar(varName string) bool {


receiver name s should be consistent with previous receiver name procStats for Stats

houndci-bot · 2017-11-28T16:40:07Z

libbeat/cmd/instance/metrics.go

+	V.OnRegistryStart()
+	defer V.OnRegistryFinished()
+
+	cpuUsage, cpuUsageNorm, totalCpuUsage, err := getCPUPercentages()


var totalCpuUsage should be totalCPUUsage

houndci-bot · 2017-11-28T16:49:23Z

libbeat/metrics/process/process.go

@@ -441,12 +451,12 @@ func isProcessInSlice(processes []Process, proc *Process) bool {

 // isWhitelistedEnvVar returns true if the given variable name is a match for
 // the whitelist. If the whitelist is empty it returns false.
-func (p ProcStats) isWhitelistedEnvVar(varName string) bool {
-	if len(p.envRegexps) == 0 {
+func (ps Stats) isWhitelistedEnvVar(varName string) bool {


receiver name ps should be consistent with previous receiver name procStats for Stats

ruflin · 2017-11-30T22:53:35Z

metricbeat/module/system/cpu/_meta/fields.yml

@@ -129,6 +129,13 @@
        The percentage of CPU time spent in non-idle state.


+    # Total


This is also needed in process as it is added for each process too.

houndci-bot · 2017-12-01T13:34:52Z

libbeat/metric/system/memory/memory.go

@@ -14,7 +11,7 @@ type MemStat struct {
 	ActualUsedPercent float64 `json:"actual_used_p"`
 }

-func GetMemory() (*MemStat, error) {
+func Get() (*MemStat, error) {


exported function Get should have comment or be unexported

houndci-bot · 2017-12-01T13:34:52Z

libbeat/metric/system/memory/memory.go

@@ -14,7 +11,7 @@ type MemStat struct {
 	ActualUsedPercent float64 `json:"actual_used_p"`
 }

-func GetMemory() (*MemStat, error) {
+func Get() (*MemStat, error) {


exported function Get should have comment or be unexported

ruflin · 2017-12-06T04:22:05Z

@kvch Please ping me when an other round of review is needed here. Let's get this in :-)

* Add both percentage and normalized percentage of CPU. * Move Round function to common.

kvch · 2018-01-04T12:59:52Z

@ruflin This PR is ready for a new review.
I removed the crosscompile jobs from Travis and opened a meta issue: #5984

tsg · 2018-01-04T13:00:56Z

.travis.yml

-      env: TARGETS="-C libbeat crosscompile"
-      go: $GO_VERSION
-      stage: test
+      #    - os: linux


Maybe leave a comment here that links to #5984

ruflin

I'm going to merge this PR and directly open a follow up PR trying to fix the crosscompile part to reenable it. Also keeping and eye on the packaging to make sure we didn't break it.

ruflin · 2018-01-04T23:50:53Z

libbeat/metric/system/memory/memory.go

@@ -1,20 +1,19 @@
-// +build darwin freebsd linux openbsd windows


What is the reason this was removed?

In elastic#5545 the crosscompile builds were disable because they didn't work with the changes. This PR reenables the crosscompiling by making the following changes: * Introduce build flags for the metrics. Currently only build on darwin, linux, windows and exclude if cgo is not available * Readd build flags to cpu and memory. They were removed in the previous PR but not sure why. * Uncomment crosscompile builds for travis This only brings back the previous behaviour. Additional changes should be made to support monitoring also on missing platforms. See elastic#5984

In #5545 the crosscompile builds were disable because they didn't work with the changes. This PR reenables the crosscompiling by making the following changes: * Introduce build flags for the metrics. Currently only build on darwin, linux, windows and exclude if cgo is not available * Readd build flags to cpu and memory. They were removed in the previous PR but not sure why. * Uncomment crosscompile builds for travis This only brings back the previous behaviour. Additional changes should be made to support monitoring also on missing platforms. See #5984

kvch added libbeat review labels Nov 8, 2017

houndci-bot reviewed Nov 8, 2017

View reviewed changes

ph suggested changes Nov 8, 2017

View reviewed changes

tsg reviewed Nov 9, 2017

View reviewed changes

ph approved these changes Nov 9, 2017

View reviewed changes

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from 9aece29 to 159661e Compare November 9, 2017 15:39

kvch added the in progress Pull request is currently in progress. label Nov 9, 2017

kvch force-pushed the feature/libbeat/cpu-usage-metric branch 2 times, most recently from 98f37b8 to 52b5634 Compare November 9, 2017 16:21

ruflin reviewed Nov 9, 2017

View reviewed changes

andrewkroh reviewed Nov 13, 2017

View reviewed changes

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from 52b5634 to e14fcb7 Compare November 14, 2017 16:46

houndci-bot reviewed Nov 14, 2017

View reviewed changes

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from e14fcb7 to 5a94f31 Compare November 14, 2017 17:16

tsullivan mentioned this pull request Nov 14, 2017

Include CPU percentage in status API elastic/kibana#11475

Closed

kvch mentioned this pull request Nov 15, 2017

Changes to support CPU usage metric in Beats elastic/gosigar#83

Merged

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from 7bdc2fd to 25cd8db Compare November 28, 2017 16:19

houndci-bot reviewed Nov 28, 2017

View reviewed changes

kvch mentioned this pull request Nov 30, 2017

Update elastic/gosigar to v0.6.0 #5775

Merged

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from a0830ef to ceebe3b Compare November 30, 2017 19:04

ruflin reviewed Nov 30, 2017

View reviewed changes

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from 986a14e to 6bbf3ff Compare December 1, 2017 10:28

houndci-bot reviewed Dec 1, 2017

View reviewed changes

kvch mentioned this pull request Dec 11, 2017

add process stubs for unsupported OSes elastic/gosigar#88

Merged

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from 2c9e5d0 to e4fc705 Compare December 12, 2017 18:14

ruflin mentioned this pull request Dec 18, 2017

Collect stats on apm-server performance elastic/apm-server#408

Closed

kvch force-pushed the feature/libbeat/cpu-usage-metric branch 3 times, most recently from 5a93f7e to 2efe38d Compare January 4, 2018 11:45

libbeat: add CPU usage to metrics

73f8e21

* Add both percentage and normalized percentage of CPU. * Move Round function to common.

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from 2efe38d to d9d63a7 Compare January 4, 2018 12:10

kvch mentioned this pull request Jan 4, 2018

Reintroduce crosscompile on tested platforms #5984

Closed

tsg reviewed Jan 4, 2018

View reviewed changes

turn off crosscompile tests

0f0f63b

kvch force-pushed the feature/libbeat/cpu-usage-metric branch from d9d63a7 to 0f0f63b Compare January 4, 2018 15:37

kvch removed the in progress Pull request is currently in progress. label Jan 4, 2018

ruflin reviewed Jan 5, 2018

View reviewed changes

ruflin merged commit e18cff5 into elastic:master Jan 5, 2018

ruflin mentioned this pull request Jan 5, 2018

Readd crosscompile builds #5993

Merged

		@@ -1,16 +1,40 @@
		// +build darwin freebsd linux openbsd windows


		// CPU Core Monitor

		// CPUCoreMonitor is used to monitor the usage of individual CPU cores.

		@@ -0,0 +1,9 @@
		package process

		// includeTopConfig is the configuration for the "top N processes

		@@ -129,6 +129,13 @@
		The percentage of CPU time spent in non-idle state.


		# Total

		@@ -1,20 +1,19 @@
		// +build darwin freebsd linux openbsd windows

New metric: CPU usage #5545

New metric: CPU usage #5545

Conversation

kvch commented Nov 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsg commented Nov 9, 2017

tsg commented Nov 9, 2017

kvch commented Nov 9, 2017

ph left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh Nov 13, 2017 • edited Loading

Choose a reason for hiding this comment

andrewkroh Nov 13, 2017 • edited Loading

Choose a reason for hiding this comment

andrewkroh Nov 13, 2017 • edited Loading

Choose a reason for hiding this comment

andrewkroh Nov 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvch commented Nov 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Dec 6, 2017

kvch commented Jan 4, 2018

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh Nov 13, 2017 •

edited

Loading

andrewkroh Nov 13, 2017 •

edited

Loading

andrewkroh Nov 13, 2017 •

edited

Loading

andrewkroh Nov 13, 2017 •

edited

Loading