Additional metrics #143

stevesloka · 2018-06-05T17:31:03Z

Add additional metrics to the Discoverers:

upstream-services
replicated-services
invalid-services
upstream-endpoints
replicated-endpoints
invalid-endpoints

// Fixes #108
Signed-off-by: Steve Sloka steves@heptio.com

rosskukulinski · 2018-06-06T17:01:14Z

In testing this I've found a few issues:

gimbal_discoverer_upstream_endpoints_total, gimbal_discoverer_replicated_endpoints_total, and invalid_endpoints should also be broken out by service with another label
On my cluster I'm seeing upstream_endpoints_total being less than replicated_endpoints_total which doesn't make sense

In addition:

Add a gimbal_discoverer_info metric which always has a value of 1 with the following labels: backendname, discoverer_version, discoverer_type
Please add a discoverer_type label to all metrics -- this would be openstack or kubernetes

rosskukulinski · 2018-06-06T18:11:15Z

Additional detail: I believe gimbal_discoverer_upstream_endpoints_total is showing currently displaying the number of Endpoint objects, not the sum of endpoints in the Endpoint objects.

rosskukulinski · 2018-06-06T19:25:32Z

gimbal_discoverer_replicated_endpoints_total doesn't seem to handle endpoints that have been removed

alexbrand

Did an initial review and added some comments. Have not tested manually yet. I wonder if there's unit tests we could build.

I am also wondering if it makes sense to keep some (or most) of these out of the sync loop, and instead start a separate goroutine that is responsible for publishing the majority of these metrics on a specific cadence (update metrics every X seconds).

It seems like they don't need to be tied to discovery events (in k8s case) or the sync loop (in openstack case).

Although now that I have written it out, I think changing to this approach could result in metrics lagging behind?

alexbrand · 2018-06-06T20:54:32Z

discovery/pkg/openstack/reconciler.go

@@ -154,7 +154,22 @@ func (r *Reconciler) reconcile() {
 		r.reconcileSvcs(desiredSvcs, currentServices.Items)

 		desiredEndpoints := kubeEndpoints(r.BackendName, projectName, loadbalancers, pools)
+		for _, ep := range desiredEndpoints {
+			desiredEndpoints = append(desiredEndpoints, ep)


Is this duplicating the list?

alexbrand · 2018-06-06T21:08:47Z

discovery/pkg/openstack/reconciler.go

+		if err != nil {
+			r.Logger.Error(err)
+		}
+		r.Metrics.DiscovererReplicatedEndpointsMetric(r.BackendName, projectName, r.sumEndpoints(endpoints.Items))


Is this competing with the metric being set in sync.Action? It's unclear to me why we need them in both places. Also seems like we are missing the equivalent for DiscovererReplicatedServicesMetric?

alexbrand · 2018-06-06T21:11:10Z

discovery/pkg/openstack/reconciler.go

+
+		// Log upstream services prometheus
+		r.Metrics.DiscovererUpstreamServicesMetric(r.BackendName, projectName, totalUpstreamServices)
+		r.Metrics.DiscovererInvalidServicesMetric(r.BackendName, projectName, totalUpstreamServices-len(loadbalancers))


nit: took me some time to figure out how this subtraction results in the number of invalid services. Can we create a variable invalidServicesCount or something and move it up closer to totalUpstreamServices and loadbalancers?

alexbrand · 2018-06-06T21:12:50Z

discovery/pkg/openstack/reconciler.go

+		endpoints, err := r.GimbalKubeClient.CoreV1().Endpoints(projectName).List(metav1.ListOptions{})
+		if err != nil {
+			r.Logger.Error(err)
+		}


Do we need an else here to avoid using endpoints when err == nil?

alexbrand · 2018-06-06T21:13:06Z

discovery/pkg/sync/service.go

+	}
+
+	// // Log Total Services Metric
+	// totalUpstreamServices, err := getTotalServicesCount(kubeClient, action.ObjectMeta().GetNamespace(), backendName)


stevesloka · 2018-06-07T13:07:18Z

@alexbrand I 100% believe we could re-do how metrics are implemented, this was my comment last time we were on the phone. I have them tied to events since that's the trigger for when something changes.

Looking into the other comments/issues now.

stevesloka · 2018-06-07T15:24:21Z

@rosskukulinski for the openstack upstream endpoints metrics. Should the servicename be the modified version we use in Gimbal or the actual value in Openstack? Right now I have it as the modified version as it matches the other metrics, but wanted to clarify.

rosskukulinski · 2018-06-07T15:26:53Z

I think modified value @stevesloka

rosskukulinski · 2018-06-07T20:28:21Z

@stevesloka I lied, the endpoint metrics should have a label which is the original service/LB name. The reason being it can be helpful to count the # endpoints available for a given Service across multiple backends.

alexbrand · 2018-06-07T19:58:55Z

discovery/cmd/kubernetes-discoverer/main.go

@@ -71,9 +71,12 @@ func main() {
 	log.Info("Gimbal Kubernetes Discoverer Starting up...")

 	// Init prometheus metrics
-	discovererMetrics = localmetrics.NewMetrics()
+	discovererMetrics = localmetrics.NewMetrics("kubernetes", backendName)


alexbrand · 2018-06-07T22:25:30Z

discovery/pkg/k8s/controller.go

+func (c *Controller) writeServiceMetrics(svc *v1.Service) error {
+	upstreamServices, err := c.serviceLister.Services(svc.GetNamespace()).List(labels.Everything())
+	if err != nil {
+		return err


Should we log instead of return here? We are not handling it at the call site

alexbrand · 2018-06-07T22:26:32Z

discovery/pkg/metrics/metrics.go

+			DiscovererUpstreamServicesGauge: prometheus.NewGaugeVec(
+				prometheus.GaugeOpts{
+					Name: DiscovererUpstreamServicesGauge,
+					Help: "Total number of services in the upstream backend cluster",


"upstream backend" sounds redundant to me. Thoughts?

alexbrand · 2018-06-07T22:28:05Z

discovery/pkg/metrics/metrics.go

+}
+
+// DiscovererInvalidEndpointsMetric records the total replicated endpoints
+func (d *DiscovererMetrics) DiscovererInvalidEndpointsMetric(namespace, serviceName string, totalEp int) {


Should we get rid of this one?

alexbrand · 2018-06-07T22:28:31Z

discovery/pkg/metrics/metrics.go

+			DiscovererInvalidEndpointsGauge: prometheus.NewGaugeVec(
+				prometheus.GaugeOpts{
+					Name: DiscovererInvalidEndpointsGauge,
+					Help: "Total number of endpoints invalid endpoints that could not be replicaed from the backend cluster",


typo: "endpoints invalid endpoints"
but not sure if we need this metric anyways.

alexbrand · 2018-06-07T22:30:41Z

discovery/pkg/openstack/reconciler.go

@@ -210,3 +219,19 @@ func (r *Reconciler) reconcileEndpoints(desired, current []v1.Endpoints) {
 		r.syncqueue.Enqueue(sync.DeleteEndpointsAction(&ep))
 	}
 }
+
+func (r *Reconciler) sumEndpoints(eps v1.Endpoints) int {


nit: can be func instead of method

alexbrand · 2018-06-07T22:30:45Z

discovery/pkg/openstack/reconciler.go

+	return total
+}
+
+func (r *Reconciler) getListFromMap(mp map[string]v1.Endpoints) []v1.Endpoints {


nit: can be func instead of method

alexbrand · 2018-06-07T22:36:13Z

discovery/pkg/sync/endpoints.go

+	metrics.EndpointsEventTimestampMetric(action.endpoints.GetNamespace(), action.endpoints.GetName(), time.Now().Unix())
+
+	// TODO: Move to lister()
+	totalEps, err := metrics.GetTotalEndpointsCount(kubeClient, action.endpoints.GetNamespace(), action.endpoints.GetName())


I am not following why we need to GET the endpoints object again, if we already have it in action.endpoints?

Good point, before I had the metric sum all endpoints for a namespace which was different.

alexbrand · 2018-06-07T22:45:09Z

discovery/pkg/metrics/metrics.go

+}
+
+// GetTotalServicesCount returns the number of services in a namespace for the particular backend
+func (d *DiscovererMetrics) GetTotalServicesCount(kubeclient kubernetes.Interface, namespace string) (int, error) {


These funcs seem somewhat out of place in that they are unrelated to the metrics package. Wondering if it makes sense to move these calls to the k8s API into the sync pkg, which is where this is called and it already has a dependency on the k8s packages.

I needed a good home for this but couldn't find one. Also, I need access to backendName to filter on labels. If it's moved from metrics will just need to make those params public.

Aaaaah I see. I think another option might be to add a backendName field to endpointsAction and serviceAction, and do AddEndpointsAction(backendName, endpoints). The downside is that we have to plumb backendName again through a couple of layers. Not sure if it's worth it at this time.

I moved stuff around, just picked the backend name out of metrics, PTAL

alexbrand

LGTM! Is it worth it for me to test it out as well?

The only other thought that just came to me is that we are adding an extra API call for every service add/update/delete we do, right?

alexbrand · 2018-06-08T14:26:29Z

discovery/pkg/metrics/metrics.go

+			DiscovererInvalidEndpointsGauge: prometheus.NewGaugeVec(
+				prometheus.GaugeOpts{
+					Name: DiscovererInvalidEndpointsGauge,
+					Help: "Total number of endpoints invalid endpoints that could not be replicaed from the backend",


typo: endpoints invalid endpoints

👍 ...also fixed replicated

stevesloka · 2018-06-08T15:00:42Z

Yes right now new api call for each service CRUD, let me see if I can pipe through the lister, save us the call.

stevesloka · 2018-06-08T19:56:46Z

ok I've got the lister piped in, so no additional api requests are required for metrics.

alexbrand · 2018-06-08T21:34:38Z

Nice! Should I test this out on my end or can we merge?

rosskukulinski · 2018-06-08T22:15:19Z

something still isn't quite right, but now it's with the service metrics.

To test, I stopped both discoverers, deleted all services in the kuard namespace in the Gimbal cluster, then launched my newly-built discoverers from your last commit.

The following has been discovered:

NAME           TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)    AGE       LABELS
blue-httpbin   ClusterIP   None         <none>        80/TCP     34s       app=httpbin,gimbal.heptio.com/backend=blue,gimbal.heptio.com/service=httpbin
blue-kuard     ClusterIP   None         <none>        8080/TCP   34s       gimbal.heptio.com/backend=blue,gimbal.heptio.com/service=kuard
green-kuard    ClusterIP   None         <none>        8080/TCP   34s       gimbal.heptio.com/backend=green,gimbal.heptio.com/service=kuard

yet here's the metrics gathered:


gimbal_discoverer_replicated_services_total{app="kubernetes-discoverer",backendname="blue",backendtype="kubernetes",cluster="k8s",instance="172.20.36.129:8080",job="kubernetes-pods",kubernetes_namespace="gimbal-discovery",kubernetes_pod_name="blue-kubernetes-discoverer-7b9999b95-rvfsc",namespace="kuard",pod_template_hash="365555651"} | 1
gimbal_discoverer_replicated_services_total{app="kubernetes-discoverer",backendname="blue",backendtype="kubernetes",cluster="k8s",instance="172.20.36.129:8080",job="kubernetes-pods",kubernetes_namespace="gimbal-discovery",kubernetes_pod_name="blue-kubernetes-discoverer-7b9999b95-rvfsc",namespace="nginx",pod_template_hash="365555651"} | 0
gimbal_discoverer_replicated_services_total{app="kubernetes-discoverer",backendname="green",backendtype="kubernetes",cluster="k8s",instance="172.20.41.62:8080",job="kubernetes-pods",kubernetes_namespace="gimbal-discovery",kubernetes_pod_name="green-kubernetes-discoverer-8496f87599-gz64x",namespace="kuard",pod_template_hash="4052943155"} | 0

rosskukulinski · 2018-06-08T22:16:22Z

And gimbal_discoverer_*_endpoints_total still has the combined servicename which has the backendname in it vs the original upstream service name.

stevesloka · 2018-06-09T00:56:34Z

I'm having trouble reproducing the service error where the counts are off. Could you port-forward to the discoverer on 8080 and double check the /metrics endpoint?

rosskukulinski · 2018-06-09T01:50:49Z

gimbal_discoverer_replicated_services_total{backendname="green",backendtype="kubernetes",namespace="kuard"} 0
gimbal_discoverer_upstream_services_total{backendname="green",backendtype="kubernetes",namespace="kuard"} 1

logs:

e="2018-06-08T22:12:05Z" level=info msg="Started workers"
time="2018-06-08T22:12:05Z" level=info msg="Successfully handled: add endpoints 'kuard/green-kuard'"
time="2018-06-08T22:12:05Z" level=info msg="Successfully handled: add service 'kuard/green-kuard'"
time="2018-06-08T22:42:05Z" level=info msg="Successfully handled: update endpoints 'kuard/green-kuard'"
time="2018-06-08T22:42:05Z" level=info msg="Successfully handled: update service 'kuard/green-kuard'"
time="2018-06-08T23:12:05Z" level=info msg="Successfully handled: update endpoints 'kuard/green-kuard'"
time="2018-06-08T23:12:05Z" level=info msg="Successfully handled: update service 'kuard/green-kuard'"
time="2018-06-08T23:42:05Z" level=info msg="Successfully handled: update endpoints 'kuard/green-kuard'"
time="2018-06-08T23:42:05Z" level=info msg="Successfully handled: update service 'kuard/green-kuard'"
time="2018-06-09T00:12:05Z" level=info msg="Successfully handled: update endpoints 'kuard/green-kuard'"
time="2018-06-09T00:12:05Z" level=info msg="Successfully handled: update service 'kuard/green-kuard'"
time="2018-06-09T00:42:05Z" level=info msg="Successfully handled: update endpoints 'kuard/green-kuard'"
time="2018-06-09T00:42:05Z" level=info msg="Successfully handled: update service 'kuard/green-kuard'"
time="2018-06-09T01:12:05Z" level=info msg="Successfully handled: update endpoints 'kuard/green-kuard'"
time="2018-06-09T01:12:05Z" level=info msg="Successfully handled: update service 'kuard/green-kuard'"
time="2018-06-09T01:42:05Z" level=info msg="Successfully handled: update endpoints 'kuard/green-kuard'"
time="2018-06-09T01:42:05Z" level=info msg="Successfully handled: update service 'kuard/green-kuard'"

rosskukulinski · 2018-06-09T01:59:05Z

@stevesloka it's possible my local container build is screwed up. If you push a build from your environment I can test that.

stevesloka · 2018-06-09T02:02:22Z

Try this image: stevesloka/gimbal-discoverer:master

stevesloka · 2018-06-09T03:05:02Z

And gimbal_discoverer_*_endpoints_total still has the combined servicename which has the backendname in it vs the original upstream service name

I thought this was only for upstream services, not all. I can work to update. I thought once you sync'd then you wanted to see the gimbal service name, not the upstream name.

stevesloka · 2018-06-12T23:59:57Z

@alexbrand @rosskukulinski I think this is ready to review again. The only things that I think could be improved are writing more tests around the Prom metrics.

Additionally, there may be cases where we need to write out "0" for some error conditions. For example, should we always write a zero for gimbal_discoverer_invalid_services_total?

I have a prebuilt image here: stevesloka/gimbal-discoverer:masterif that helps.

Some notable changes:

Openstack discoverer uses a custom struct now (Endpoints) which allows it to store the v1.Endpoints along with the upstream` service name.
Sync package seperates out the process from metrics piece to make that a bit cleaner.
The syncd services metric uses a query to the api server to accomplish. I think this is reasonable as services shouldn't update as frequently as endpoints. I have some ideas on ways to stream line but it changes the structure quite a bit and I think we should discuss first.

rosskukulinski · 2018-06-13T16:41:49Z

@stevesloka This is now working for me! I don't think we need to write out a zero for metrics that don't have a value.

alexbrand · 2018-06-14T13:57:09Z

discovery/cmd/kubernetes-discoverer/main.go

@@ -46,7 +46,7 @@ var (

 func init() {
 	flag.BoolVar(&printVersion, "version", false, "Show version and quit")
-	flag.IntVar(&numProcessThreads, "num-threads", 2, "Specify number of threads to use when processing queue items.")
+	flag.IntVar(&numProcessThreads, "num-threads", 1, "Specify number of threads to use when processing queue items.")


Any reason for changing this?

nah, just local debugging, I'll restore

alexbrand · 2018-06-14T13:58:29Z

discovery/cmd/openstack-discoverer/main.go

@@ -50,6 +50,7 @@ var (
 	prometheusListenPort              int
 	discovererMetrics                 localmetrics.DiscovererMetrics
 	log                               *logrus.Logger
+	resyncInterval                    time.Duration


Doesn't look like this is used

alexbrand · 2018-06-14T14:04:06Z

discovery/pkg/k8s/controller_test.go

+
+			gathering, err := gatherers.Gather()
+			if err != nil {
+				fmt.Println(err)


t.Fatal here?

alexbrand · 2018-06-14T14:05:36Z

discovery/pkg/k8s/controller_test.go

+
+			gathering, err := gatherers.Gather()
+			if err != nil {
+				fmt.Println(err)


t.Fatal here?

alexbrand · 2018-06-14T14:10:01Z

discovery/pkg/openstack/reconciler.go

+		// Convert the k8s list to type []Endpoints so make comparison easier
+		currentEndpoints := []Endpoints{}
+		for _, v := range currentk8sEndpoints.Items {
+			currentEndpoints = append(currentEndpoints, Endpoints{endpoints: v, upstreamName: ""})


Do we need to set the upstreamName here?

Those are the current endpoints which come from the k8s query. The upstream name is not used, but having two lists of the same type makes the comparison logic work as-is.

alexbrand · 2018-06-14T14:23:41Z

discovery/pkg/sync/endpoints_test.go

+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			nowFunc = func() time.Time {


How/where is this used?

The Timestamp metric used time.Now() to get the current time. In order to unit test that metric, we need a way to set it in the test. If it's not set in the test, then it defaults to now(). Both the service & endpoint in package sync use this.

alexbrand · 2018-06-14T14:25:05Z

discovery/pkg/sync/endpoints_test.go

+		expectErr         bool
+		expectedCount     float64
+		expectedTimestamp float64
+		expectedLabels    map[string]string


Doesn't look like there are assertions on the labels

No, I didn't write tests for those. They are simple pass-through values. But initially, I had though to implement those.

Got it! Should we remove them from this struct?

alexbrand · 2018-06-14T14:28:18Z

discovery/pkg/sync/queue.go

-	err := action.Sync(sq.KubeClient, sq.Metrics, sq.BackendName)
+	err := action.Sync(sq.KubeClient, sq.Logger)
+	if err != nil {
+		sq.Metrics.ServiceMetricError(action.ObjectMeta().GetNamespace(), action.ObjectMeta().GetName(), action.GetActionType())


What if we get an error while handling an Endpoints resource?

yup, meant to refactor that, good catch.

alexbrand · 2018-06-14T14:29:27Z

discovery/pkg/sync/endpoints.go

 	return nil
 }

 func (action endpointsAction) String() string {
 	return fmt.Sprintf(`%s endpoints '%s/%s'`, action.kind, action.endpoints.Namespace, action.endpoints.Name)
 }

-func addEndpoints(kubeClient kubernetes.Interface, endpoints *v1.Endpoints, lm localmetrics.DiscovererMetrics, backendName string) error {
+func (action endpointsAction) LogMetrics(gimbalKubeClient kubernetes.Interface, metrics localmetrics.DiscovererMetrics,


nit: Can we call this SetMetrics or UpdateMetrics? Log makes me think of log messages

How about WriteMetrics? Set & Update sound like they are values that are getting set.

eh we can do SetMetrics I think

alexbrand · 2018-06-14T14:33:03Z

discovery/pkg/sync/time.go

+
+var nowFunc nowFuncT
+
+func init() {


Can we add some docs on why we need this and what it is doing?

Just inline comments? We don't have any dev docs at the moment.

Yep, sorry! That is what I meant :)

I cleaned it up, removed unused methods, added the missing header, and added some comments.

alexbrand · 2018-06-14T20:15:15Z

discovery/pkg/sync/time.go

+// a way to override the default values.
+type nowFuncT func() time.Time
+
+var nowFunc nowFuncT


I think this will result in a nil-pointer deref outside of tests

yeah my bad, I got overzealous, I restored the old impl.

alexbrand · 2018-06-14T20:16:09Z

discovery/pkg/openstack/translate.go

@@ -114,6 +114,15 @@ func serviceName(lb loadbalancers.LoadBalancer) string {
 	return strings.ToLower(lbName)
 }

+// get the lb Name or ID if name is empty
+func serviceNameOriginal(lb loadbalancers.LoadBalancer) string {


Will this be a problem if there are two load balancers with the same name? Seems like it'll affect the metrics, but not 100% sure.

Yes, it will. @rosskukulinski do you want to allow duplicates here? It only should affect openstack at the moment.

The alternative is to use the name + ID, but not sure if that is what we want.

TBH I'm really not sure. I don't know how often folks name their load balancers the same thing in the same cluster. Let's leave it as-is for now (at risk of duplicates).

If the LB doesn't have a name, do we put in the id for the label?

yes name is tried first, then id is used.

stevesloka · 2018-06-15T01:48:54Z

Testing tonight, I lost the golang metrics, looking to add those back in.

stevesloka · 2018-06-15T02:12:17Z

Ok latest commit adds those metrics back in.

…ervices, upstream-endpoints, replicated-endpoints, invalid-endpoints) Signed-off-by: Steve Sloka <steves@heptio.com>

alexbrand

There's a couple of comments that might require some follow up work, but I still want to merge this to start testing. I will open up issues for us to follow up.

alexbrand · 2018-06-18T12:19:28Z

discovery/cmd/openstack-discoverer/main.go

@@ -71,6 +72,7 @@ func init() {
 	flag.IntVar(&prometheusListenPort, "prometheus-listen-address", 8080, "The address to listen on for Prometheus HTTP requests")
 	flag.Float64Var(&gimbalKubeClientQPS, "gimbal-client-qps", 5, "The maximum queries per second (QPS) that can be performed on the Gimbal Kubernetes API server")
 	flag.IntVar(&gimbalKubeClientBurst, "gimbal-client-burst", 10, "The maximum number of queries that can be performed on the Gimbal Kubernetes API server during a burst")
+	flag.DurationVar(&resyncInterval, "resync-interval", time.Minute*30, "Default resync period for watcher to refresh")


Looks like this isn't used. I want to merge this PR to start testing, so I will open a new issue to discuss this flag. It seems like we can remove it, but wanted to confirm with Steve.

alexbrand · 2018-06-18T12:40:57Z

discovery/pkg/sync/queue.go

+	if err != nil {
+		action.SetMetricError(sq.Metrics)
+	}
+	action.SetMetrics(sq.KubeClient, sq.Metrics, sq.Logger)


I am wondering how these metrics will behave if we need to retry. Is it OK to set these metrics multiple times for the same service because of retries? Will open a new issue to discuss/look into this.

rosskukulinski requested review from rosskukulinski and alexbrand and removed request for rosskukulinski June 6, 2018 16:20

alexbrand reviewed Jun 6, 2018

View reviewed changes

stevesloka force-pushed the addMetrics branch from 1171788 to faa5221 Compare June 7, 2018 19:15

rosskukulinski mentioned this pull request Jun 7, 2018

Updated Grafana Discovery dashboard using new discoverer metrics #147

Merged

alexbrand reviewed Jun 7, 2018

View reviewed changes

alexbrand reviewed Jun 8, 2018

View reviewed changes

stevesloka force-pushed the addMetrics branch from 2d3b14b to 7bdb92a Compare June 8, 2018 20:59

stevesloka force-pushed the addMetrics branch from 20a72b8 to 8aa793c Compare June 12, 2018 23:47

alexbrand reviewed Jun 14, 2018

View reviewed changes

stevesloka force-pushed the addMetrics branch from 39f52f7 to 144c240 Compare June 15, 2018 14:35

Additional metrics (upstream-services, replicated-services, invalid-s…

991b654

…ervices, upstream-endpoints, replicated-endpoints, invalid-endpoints) Signed-off-by: Steve Sloka <steves@heptio.com>

stevesloka force-pushed the addMetrics branch from 144c240 to 991b654 Compare June 15, 2018 14:39

alexbrand approved these changes Jun 18, 2018

View reviewed changes

alexbrand merged commit 314734a into projectcontour:master Jun 18, 2018

This was referenced Jun 18, 2018

OpenStack discoverer: resync interval flag is not used #153

Closed

Discoverer metrics: Handling of metrics when we have to retry sync operations #154

Closed

stevesloka deleted the addMetrics branch June 25, 2018 13:13

Additional metrics #143

Additional metrics #143

Conversation

stevesloka commented Jun 5, 2018

rosskukulinski commented Jun 6, 2018 • edited Loading

rosskukulinski commented Jun 6, 2018 • edited Loading

rosskukulinski commented Jun 6, 2018

alexbrand 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

stevesloka commented Jun 7, 2018

stevesloka commented Jun 7, 2018

rosskukulinski commented Jun 7, 2018

rosskukulinski commented Jun 7, 2018

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

alexbrand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevesloka commented Jun 8, 2018

stevesloka commented Jun 8, 2018

alexbrand commented Jun 8, 2018

rosskukulinski commented Jun 8, 2018

rosskukulinski commented Jun 8, 2018

stevesloka commented Jun 9, 2018

rosskukulinski commented Jun 9, 2018

rosskukulinski commented Jun 9, 2018

stevesloka commented Jun 9, 2018

stevesloka commented Jun 9, 2018 • edited Loading

stevesloka commented Jun 12, 2018 • edited Loading

rosskukulinski commented Jun 13, 2018

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

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

stevesloka commented Jun 15, 2018

stevesloka commented Jun 15, 2018

alexbrand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rosskukulinski commented Jun 6, 2018 •

edited

Loading

rosskukulinski commented Jun 6, 2018 •

edited

Loading

stevesloka commented Jun 9, 2018 •

edited

Loading

stevesloka commented Jun 12, 2018 •

edited

Loading