Refactored version of collector #65

marcin-krolik · 2017-01-04T15:23:21Z

Summary of changes:

removed dependency to openlibcontainers/runc
re-factored metric collection process, only requested metrics are gathered per requested docker id
proper discovery of cgroupfs

TODO:

PatrykMatyjasek · 2017-01-05T10:18:32Z

collector/utils.go

+func initClient(c *collector, endpoint string) error {
+	dc, err := container.NewDockerClient(endpoint)
+	if err != nil {
+		return err


missing log

logs moved to collector level. This package only returns errors now

PatrykMatyjasek · 2017-01-05T10:18:47Z

collector/utils.go

@@ -0,0 +1,164 @@
+package collector


missing header

header added

PatrykMatyjasek · 2017-01-05T10:24:21Z

container/client.go

+		fields := strings.Split(txt, " ")
+		for _, opt := range strings.Split(fields[len(fields)-1], ",") {
+			if opt == subsystem {
+				return fields[4], nil


possible index out of range?

It would require to /proc/self/mountinfo change its format. I think it's not needed

PatrykMatyjasek · 2017-01-05T10:28:02Z

container/fs/blkio.go

+			}
+		}
+
+		v, err := strconv.ParseUint(fields[0], 10, 64)


maybe use "major" instead of "v"?

PatrykMatyjasek · 2017-01-05T10:28:14Z

container/fs/blkio.go

+		}
+		major := v
+
+		v, err = strconv.ParseUint(fields[1], 10, 64)


maybe use "minor" instead of "v"?

PatrykMatyjasek · 2017-01-05T10:42:03Z

container/fs/pids.go

+
+	stats.Cgroups.PidsStats.Current = current
+
+	var max uint64


max shouldn't be initialized with some default value? What will happen if limit = "max"?

It is initialized to default value of uint64 which is 0. For limit equal to zero, it means no limit.

What will happen if limit = "max"?

In this case limit will be set to defult value of max

PatrykMatyjasek · 2017-01-05T10:45:19Z

container/network/tcp.go

+	if err != nil {
+		return err
+	}
+	fmt.Println(pid, isHost, procfs)


unnecessary print

PatrykMatyjasek · 2017-01-05T10:46:42Z

container/client.go

+http://www.apache.org/licenses/LICENSE-2.0.txt
+
+
+Copyright 2015 Intel Corporation


2017, also in other files? :)

I think it was agreed before dates in header files don't have to be updated while introducing changes to already released plugin.

kindermoumoute · 2017-01-05T17:17:35Z

collector/collector.go

+	return *policy, nil
+}
+
+type collector struct {


Can we move this type definition on the top, after const definition?

It's unexported type. It's quite common code style to keep unexported types below exported ones

+1 to @kindermoumoute's comment because this is a type which is returned by New() in line 94, so it will be great to have a definition of this structure after const or variables (which are also unexported but are placed on the top despite this).

New() now returns interface. collector type as unexported type stays behind exported API

ok, that is fine for me.

kindermoumoute · 2017-01-05T17:24:02Z

collector/collector.go

+		} // the end of range over ids
+	}
+
+	if len(metrics) == 0 {


This should be the first thing to test.

I don't think so. It tests if metrics collection process actually collected anything

My bad, I thought you were catching if len(mts) == 0. This case should never be allowed by the framework though.

IzabellaRaulin · 2017-01-10T12:39:21Z

collector/collector.go

+	// namespace plugin name
+	PLUGIN_NAME = "docker"
+	// version of plugin
+	PLUGIN_VERSION = 5


please, increment the version of plugin

Version increased to 6

IzabellaRaulin · 2017-01-10T14:14:37Z

collector/collector.go

+	return &collector{
+		containers: map[string]*container.ContainerData{},
+		mounts:     map[string]string{},
+	}, nil


After this refactoring, returning error from New() is no longer needed - returned err is always nil. Consider removing error from output and updating go-doc for this function

IzabellaRaulin · 2017-01-10T14:29:14Z

collector/collector.go

+		c.conf, err = getDockerConfig(mts[0].Config)
+		if err != nil {
+			log.WithFields(log.Fields{
+				"plugin": "docker_collector",


Adding "plugin" as log's field duplicates information added by framework, so this field might be removed.

fixed logging in whole module

IzabellaRaulin · 2017-01-11T13:18:11Z

container/network/tcp.go

+	}
+
+	return nil
+}


Notice that tcp metrics and tcp6 metrics are exactly the same. what is more the way of retrieving these data differs only in the taken path. For this refactored code distinguishing between tcp and tcp is made on defined structures, please consider putting all this code from GetStats in different function with input argument pointing to "net/tcp" or "net/tcp6" depending on a case and use tcpStatsFromProc directly, so functions TcpStatsFromProc and Tcp6StatsFromProc are not longer needed.

Thanks for changing that, now it looks good

IzabellaRaulin · 2017-01-11T13:39:50Z

container/client.go

+
+	if len(containers) == 0 {
+		return nil, fmt.Errorf("No docker containers found %d", len(containers))
+	}


I noticed that this if statement does not have sense because this containers map always contains "root". So, what do you think about moving this statement before adding "root" to this map, but only logging this fact as an "INFO" or "DEBUG", because even there is no docker containers, collection should not fail as it works currently (in a sense by accident).

Thanks for moving it up and adding a log, now it looks good

IzabellaRaulin · 2017-01-12T10:10:22Z

METRICS.md

+hugetlb_stats/\<size\>/max_usage | uint64 | Max "hugepagesize" hugetlb  usage recorded
+hugetlb_stats/\<size\>/usage | uint64 | Current usage for "hugepagesize" hugetlb
+| |
+blkio_stats/io_merged_recursive/\<N\>/value | uint64 | Total number of bios/requests merged into requests belonging from all the descendant cgroups 


@marcin-krolik, what's about major, minor, and op for blkio_stats?

On the occasion if you could remove metric "network/name" which is no longer exposed (because of including name as a dynamic element), it will be great :)

great, but @marcin-krolik - could you make descriptions of these major, minor, and op blkio_stats consistent with the rest? Also, a new line might be put between these points:
1) Hierarchical version of cgroups counter which in addition to the cgroup's own value includes the sum of all hierarchical children's values (2) Each blkio statistic additionally exposes major, minor and op metric

IzabellaRaulin · 2017-01-12T10:14:42Z

METRICS.md

 cpu_stats/throttling_data/throttled_time | uint64 | The total time duration for which tasks in a cgroup have been throttled
+cpu_stats/cpu_shares | uint64 | contains an integer value that specifies a relative share of CPU time available to the tasks in a cgroup


To keep consistency, consider changing contains an integer value that specifies a relative share of CPU time available to the tasks in a cgroup to The relative share of CPU time available to the tasks in a cgroup. Please, take into account the rest of similar cases.

IzabellaRaulin · 2017-01-12T10:33:48Z

METRICS.md

+blkio_stats/io_serviced_recursive/\<N\>/value | uint64 | Number of IOs (bio) issued to the disk from all the descendant cgroups
+blkio_stats/io_time_recursive/\<N\>/value | uint64 | Disk time allocated to cgroup per device in milliseconds from all the descendant cgroups
+blkio_stats/io_wait_time_recursive/\<N\>/value | uint64 | Total amount of time the IOs for this cgroup spent waiting in the scheduler queues for service from all the descendant cgroups
+blkio_stats/sectors_recursive/\<N\>/value | uint64 | number of sectors transferred to/from disk from all descendant group

 <sup>(1)</sup> Hierarchical version of cgroups counter which in addition to the cgroup's own value includes the sum of all hierarchical children's values



Would you like to update the link to Kernel cgroups documentation, currenty it does not work.
So METRICS.md, line 131 should be changed to:
Read more about cgroups in [Kernel documentation](https://www.kernel.org/doc/Documentation/cgroup-v1/cgroups.txt)

IzabellaRaulin · 2017-01-12T11:10:48Z

collector/collector.go

+	cgroupfs   string
+	driver     string
+	mounts     map[string]string
+	conf       map[string]string


Coud you add comments to the rest of stucture's fields as it's done for "containers" and "client"?

IzabellaRaulin · 2017-01-12T11:11:56Z

collector/collector.go

+		}
+
+		for group := range groups {
+			// during initialization of docker client information about running containers is collectd


"collectd" probably to collected"

ok, now it is ok

IzabellaRaulin · 2017-01-12T11:15:31Z

collector/collector.go

+
+				if rid != "root" {
+					if c.cgroupfs == "systemd" {
+						cpath = filepath.Join(cpath, "system.slice", fmt.Sprintf("docker-%s.scope", c.containers[rid].ID))


What do you think about adding here a log with information about the path to cgroups? I suppose it will be helfpul based on issues which were reported in the past.

After changing this fragment of code, there is still lack of logging of cgroup paths - I suppose that based on past issues putting a log there will be very helpful.

IzabellaRaulin · 2017-01-12T11:16:26Z

collector/collector.go

+			// only log error when it was not possible to access metric source
+			if err != nil {
+				log.WithFields(log.Fields{
+					"plugin": "docker_collector",


duplication - you do not need "plugin": "docker_collector" in logs, because Snap's framework will add that.

IzabellaRaulin · 2017-01-12T11:21:35Z

collector/collector_test.go

@@ -367,38 +369,10 @@ func TestCollectMetrics(t *testing.T) {
 				})
 			})
 		})


Would you like to add tests covering cases for filesystem metrics and alsonew ones (hugetlb, blkio and cpuset statistics)?

I think it could be delivered in separate PR

ok, that works for me

IzabellaRaulin · 2017-01-12T11:24:42Z

collector/utils.go

+	c.driver = drv
+
+	log.WithFields(log.Fields{
+		"plugin": "docker_collector",


This field "plugin": "docker_collector" might be removed.

ok, now it looks great

IzabellaRaulin · 2017-01-12T16:10:38Z

collector/utils.go

+
+func getDockerConfig(cfg plugin.Config) (map[string]string, error) {
+	config := make(map[string]string)
+	values := []string{"endpoint", "procfs"}


Does it make sense to declare strings "endpoint" and " procfs" as a constant? Also, if this getDockerConfig function is added, maybe checking all required configs will be better and then return combined error or []error.

These strings are used only once, I don't see any point in adding them as constants.

IzabellaRaulin · 2017-01-12T16:13:51Z

collector/utils.go

+	values := []string{"endpoint", "procfs"}
+	var err error
+	for _, v := range values {
+		config[v], err = getStringFromConfig(cfg, v)


Could you explain why cfg.GetString(key) is not called directly, but a new function getStringFromConfig was used for do it?

It should be. Changed to direct call

IzabellaRaulin · 2017-01-12T16:30:41Z

container/cgroupfs/blkio.go

+		}
+
+		op := ""
+		valueField := 2


Light suggestion - consider renaming valueField to indexOfFieldValue or sth similar that clearly suggest that this is an index, not a value itself.

changed to index

IzabellaRaulin · 2017-01-12T16:37:02Z

container/cgroupfs/blkio.go

+}
+
+func getCFQStats(path string, stats *container.Statistics) error {
+	var blkioStats []container.BlkioStatEntry


Light suggestion - consider renaming blkioStats to blkioStatEntry to highlight that this is only a part of blkio stats, of course up to you.

These are single entries, and each entry is blkio statistic, quite descriptive. I don't see the need to change that

IzabellaRaulin · 2017-01-12T16:40:20Z

container/cgroupfs/blkio.go

+	if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.sectors_recursive")); err != nil {
+		return err
+	}
+	stats.Cgroups.BlkioStats.SectorsRecursive = blkioStats


Consider changing this statement:

if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.sectors_recursive")); err != nil { return err } stats.Cgroups.BlkioStats.SectorsRecursive = blkioStats

to

if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.sectors_recursive")); err == nil { stats.Cgroups.BlkioStats.SectorsRecursive = blkioStats } else { errs=append(errs, err) }

and returning []error with error messages from all called getBlkioStat

With current implementation it fails early with first error. What would be an advantage of your approach?

The main advantage of this approach is informing a user about all blkiostats errors in a first run, but I can agree that for this case it does not matter so much. However, this is really helpful in such cases like checking if config options are correct - that allows avoiding many runs with fails.

So, it looks ok

IzabellaRaulin · 2017-01-12T16:57:57Z

container/cgroupfs/blkio.go

+	if blkioStats, err := getBlkioStat(filepath.Join(path, "blkio.io_serviced_recursive")); err == nil && blkioStats != nil {
+		return getCFQStats(path, stats)
+	}
+	return getStats(path, stats) // Use generic stats as fallback


Could you put here (or above functions getCFQStats and getStats ) what distinguishes them - I mean why, based on on availability of blkio.io_serviced_recursive, one of them is called, otherwise the second one.

It is already stated in comment from line 44 // Try to read CFQ stats available on all CFQ enabled kernels first and then // Use generic stats as fallback in line 48

At first glance, these comments were not clear for me, but thanks for explanation. 👍

marcintao · 2017-01-13T09:16:18Z

container/fs/fs.go

@@ -200,17 +206,38 @@ func (self *RealFsInfo) GetFsInfoForPath(mountSet map[string]struct{}) ([]Fs, er
 }

 // GetFsStats returns filesystem statistics for a given container
-func GetFsStats(container *docker.Container) (map[string]wrapper.FilesystemInterface, error) {
+func (du *DiskUsageCollector) GetStats(stats *container.Statistics, opts container.GetStatOpt) error {


Right now GetStats() routine is basing on a constant path to storage mount point, /var/lib/docker. It might be good idea to discover this path while initializing docker client with call to GetDockerParam("DockerRootDir") - see Docker API /info endpoint.

Good catch. Thanks!

IzabellaRaulin · 2017-01-13T09:43:03Z

collector/utils.go

+		return err
+	}
+
+	drv, err := dc.GetDockerParam("Driver")


Consider change it to c.driver, err = dc.GetDockerParam("Driver"), so it will exactly the same way how it's done above`

IzabellaRaulin · 2017-01-13T09:43:59Z

collector/utils.go

+	log.WithFields(log.Fields{
+		"plugin": "docker_collector",
+		"block":  "initClient",
+	}).Infof("Docker client initialized with %s and %s drivers", drv, c.cgroupfs)


Consider changing drv to c.driver in this info-log message

IzabellaRaulin · 2017-01-13T10:00:05Z

container/cgroupfs/cpu.go

+
+// GetStats reads throttling metrics from Cpu Group from cpu.stat
+func (cpu *Cpu) GetStats(stats *container.Statistics, opts container.GetStatOpt) error {
+	path, err := opts.GetStringValue("cgroup_path")


I have a concern about calling opts.GetStringValue("cgroup_path") in each of GetStats functions. Putting cgroup_path as an input argument instead of opts might have more sense, so these GetStats function will be delacred in the following way:
func (cpu *Cpu) GetStats(stats *container.Statistics, cgroup_path string) error
func (cpuacct *CpuAcct) GetStats(stats *container.Statistics, cgroup_path string) error
func (cpuShares *CpuShares) GetStats(stats *container.Statistics, cgroup_path container.GetStatOpt) error

I noticed that for network GetStats not only cgroup_path is needed, but also others fields from opts, so please ignore my comment above.

This will complicate interface. For other groups then cgroups getters other sets of parameters is required. With opts it easier to handle different cases

yes, I agree with that

IzabellaRaulin · 2017-01-13T10:07:40Z

container/cgroupfs/hugetlb.go

+		nameArray := strings.Split(st.Name(), "-")
+		pageSize, err := units.RAMInBytes(nameArray[1])
+		if err != nil {
+			return []string{}, err


return nil, err

IzabellaRaulin · 2017-01-13T10:09:19Z

container/cgroupfs/hugetlb.go

+	}
+	for _, st := range files {
+		nameArray := strings.Split(st.Name(), "-")
+		pageSize, err := units.RAMInBytes(nameArray[1])


@marcin-krolik, please verify if this nameArray[1] does not cause potential out-of -range error in case when st.Name() does not contain "-" and splitting returns only one element.

Such situation would require changes in structure of kernel file

IzabellaRaulin · 2017-01-13T10:26:40Z

container/cgroupfs/utils.go

+		return 0, err
+	}
+
+	return strconv.ParseUint(strings.Trim(string(raw), "\n"), 10, 64)


Consider changing strings.Trim(string(raw), "\n") to strings.TrimSpace(string(raw)) what will remove "\n" and others white spaces

IzabellaRaulin · 2017-01-13T10:37:52Z

container/cgroupfs/utils.go

+
+	value, err = strconv.ParseUint(fields[1], 10, 64)
+	if err != nil {
+		return name, value, err


What do you think about adding to error message some info which will be helpful to point which value cannot be parsed to uint, for example:

fmt.Errorf("Cannot parse value for field %s, err=%s", field[0], err.Error())

or maybe put some log- it might be here or in a place where parseEntry is called.

ParseUint error already contains such information eg. strconv.ParseUint: parsing "33f": invalid syntax

IzabellaRaulin · 2017-01-13T10:48:55Z

container/cgroupfs/hugetlb.go

+		hugetlbStats.MaxUsage = maxUsage
+		hugetlbStats.Failcnt = failcnt
+
+		stats.Cgroups.HugetlbStats[pageSize] = hugetlbStats


Small suggestion: consider removing hugetlbStats var in this case, because it's not really needed. It might be implemented in the following way:

stats.Cgroups.HugetlbStats[pageSize] = container.HugetlbStats{ Usage: usage, MaxUsage: maxUsage, Failcnt: failcnt, }

but up to you - if you prefer leave it as it is now, that's ok for me too

IzabellaRaulin · 2017-01-13T11:41:31Z

container/cgroupfs/memory.go

+		return err
+	}
+
+	if val, ok := stats.Cgroups.MemoryStats.Stats["cache"]; ok {


Consider adding a comment to explain why stats.Cgroups.MemoryStats.Stats is checked as a first to get cache value.

IzabellaRaulin · 2017-01-13T11:44:25Z

container/cgroupfs/memory.go

+// MemoryCache implements StatGetter interface
+type MemoryCache struct{}
+
+// GetStats reads memory cache metric from Memory Group from memory.stat


Please, verify if this description is really equivalent to how this func works

it is, cache value is taken from memory.stat file.

IzabellaRaulin · 2017-01-13T11:50:52Z

container/cgroupfs/pids.go

+
+	var max uint64
+	if limit != "max" {
+		max, err = strconv.ParseUint(strings.Trim(string(limit), "\n"), 10, 64)


Consider using strings.TrimSpace(string(limit)) instead

IzabellaRaulin · 2017-01-13T11:56:09Z

container/client.go

+
+// DockerClient holds fsouza go-dockerclient instance ready for communication with the server endpoint `unix:///var/run/docker.sock`,
+// cache instance which is used to store output from docker container inspect (to avoid execute inspect request multiply times, it is called only once per container)
+// and diskUsageCollector which is responsible for collecting container disk usage (based on `du -u` command) in the background


Please check if this comment is up to date - I noticed that diskUsageCollector is no longer in DockerClient.

IzabellaRaulin · 2017-01-13T11:58:20Z

container/client.go

+	return info, nil
+}
+
+// GetDockerParam returns given parameter value fro running docker engine


fro should be from

IzabellaRaulin · 2017-01-13T12:05:03Z

container/client.go

+
+// GetDockerParam returns given parameter value fro running docker engine
+func (dc *DockerClient) GetDockerParam(param string) (string, error) {
+	env, err := dc.cl.Info()


What do you think about changing it to GetDockerParams and allowing to make a request for a few params, so dc.cl.Info() will be executed only once. Does it have a sense?

After change, it looks great

IzabellaRaulin · 2017-01-13T14:17:30Z

container/client.go

+
+	for scanner.Scan() {
+		txt := scanner.Text()
+		fields := strings.Split(txt, " ")


Small suggestion - consider using here strings.Fields(txt)

IzabellaRaulin · 2017-01-13T14:17:54Z

container/client.go

+	scanner := bufio.NewScanner(f)
+	for scanner.Scan() {
+		txt := scanner.Text()
+		fields := strings.Split(txt, " ")


Small suggestion - consider using here strings.Fields(txt)

IzabellaRaulin · 2017-01-13T14:22:00Z

container/fs/fs.go

@@ -52,11 +54,12 @@ type RealFsInfo struct {
 	labels map[string]string
 }

-var collector DiskUsageCollector
+//var collector DiskUsageCollector

 type DiskUsageCollector struct {


Consider re-use here the comment from client.go#L5 about DiskUsageCollector

IzabellaRaulin · 2017-01-13T14:51:03Z

container/network/tcp.go

+				"module": "network",
+				"block":  "GetStats",
+			}).Errorf("Unknown tcp stats file %s", tcp.StatsFile)
+			return err


Here a new error should be created and returned, consider put here return fmt.Errorf("Unknown tcp stats file %s", tcp.StatsFile"). At the moment it looks like the last err's value is returned what means "nil".

IzabellaRaulin · 2017-01-13T15:05:27Z

container/statistics.go

+		return val.(int), nil
+	}
+
+	return 1, fmt.Errorf("could not find value for key %s", key)


Why does it return 1? Returning 0 seems to be more appropriate, but up to you.

IzabellaRaulin · 2017-01-13T15:24:58Z

mocks/mocks.go


-	return stats
+func (m *MockTcp6) GetStats(stats *container.Statistics, opts container.GetStatOpt) error {


Mocking for Tcp6 might be removed after TCP6 structure is no longer used

IzabellaRaulin · 2017-01-13T15:42:27Z

container/statistics.go

+func (opt GetStatOpt) GetStringValue(key string) (string, error) {
+	val, exists := opt[key]
+	if exists {
+		return val.(string), nil


Consider adding checking the result of type assertion. It might look like below:

func (opt GetStatOpt)getStringValue(key string)(string, error) { if val, exists := opt[key]; exists { if valStr, ok := val.(string); ok { return valStr, nil } return "", fmt.Errorf("Value for key `%s` is expected to be string, but it isn't", key) } return "", fmt.Errorf("Cannot find value for key `%s`", key) }

check added

IzabellaRaulin

LGTM

PatrykMatyjasek suggested changes Jan 5, 2017

View reviewed changes

kindermoumoute reviewed Jan 5, 2017

View reviewed changes

IzabellaRaulin reviewed Jan 10, 2017

View reviewed changes

PatrykMatyjasek approved these changes Jan 10, 2017

View reviewed changes

marcin-krolik force-pushed the feat/refactor branch from 64567f6 to afff4f3 Compare January 11, 2017 13:42

IzabellaRaulin suggested changes Jan 11, 2017

View reviewed changes

IzabellaRaulin reviewed Jan 12, 2017

View reviewed changes

marcintao reviewed Jan 13, 2017

View reviewed changes

IzabellaRaulin reviewed Jan 13, 2017

View reviewed changes

marcin-krolik force-pushed the feat/refactor branch 7 times, most recently from 10c63e1 to 40c85d5 Compare January 19, 2017 11:11

marcin-krolik changed the title ~~WIP: refactored version of collector~~ Refactored version of collector Jan 19, 2017

IzabellaRaulin approved these changes Jan 19, 2017

View reviewed changes

marcin-krolik force-pushed the feat/refactor branch from 03b56f9 to 571b736 Compare January 20, 2017 10:35

Refactored version of docker collector

013199d

marcin-krolik force-pushed the feat/refactor branch from 1bc4589 to 013199d Compare January 20, 2017 12:41

marcin-krolik merged commit 350969e into intelsdi-x:master Jan 20, 2017

		http://www.apache.org/licenses/LICENSE-2.0.txt


		Copyright 2015 Intel Corporation

		cpu_stats/throttling_data/throttled_time \| uint64 \| The total time duration for which tasks in a cgroup have been throttled
		cpu_stats/cpu_shares \| uint64 \| contains an integer value that specifies a relative share of CPU time available to the tasks in a cgroup

@@ @@ -367,38 +369,10 @@ func TestCollectMetrics(t *testing.T) { @@
               				})
               			})
               		})


		return stats
		func (m MockTcp6) GetStats(stats container.Statistics, opts container.GetStatOpt) error {

Refactored version of collector #65

Refactored version of collector #65

Conversation

marcin-krolik commented Jan 4, 2017 • edited Loading

Choose a reason for hiding this comment

marcin-krolik Jan 5, 2017 • edited Loading

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

marcin-krolik Jan 5, 2017 • edited Loading

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

IzabellaRaulin Jan 19, 2017 • edited Loading

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

IzabellaRaulin Jan 12, 2017 • edited Loading

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

IzabellaRaulin Jan 12, 2017 • edited Loading

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

IzabellaRaulin Jan 19, 2017 • edited Loading

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

IzabellaRaulin Jan 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcin-krolik commented Jan 4, 2017 •

edited

Loading

marcin-krolik Jan 5, 2017 •

edited

Loading

marcin-krolik Jan 5, 2017 •

edited

Loading

IzabellaRaulin Jan 19, 2017 •

edited

Loading

IzabellaRaulin Jan 12, 2017 •

edited

Loading

IzabellaRaulin Jan 12, 2017 •

edited

Loading

IzabellaRaulin Jan 19, 2017 •

edited

Loading

IzabellaRaulin Jan 12, 2017 •

edited

Loading

IzabellaRaulin Jan 19, 2017 •

edited

Loading