Basic workers stats by karitra · Pull Request #148 · interiorem/stout

karitra · 2018-01-16T10:09:20Z

For preliminary review.

Gopher gurus, @3Hren, @antmat, PTAL.

Metrics (for porto and procfs):

cpu load: percents
memory: bytes count
network: tx, rx bytes count (for porto only, but yet system wide stat provided)

Setup (via config file)

{
...
  "isolate": { 
      "porto" {
            "type": "porto",
            "args": {
                "workersmetrics": {
                    "period": "5s"
                }
            }
        },
        "process": {
            "type": "process",
            "args": {
                "workersmetrics": {
                    "period": "5s"
                }
            }
        },
       ...
  }
}

Zero poll period (default) switches metrics poll off.

TODO:

per worker network stat (seems not trivial to implement)
docker isolation stats (not in priority, separate PR)
tests

+ show uuids metrics query in debug log

- (porto) cpu load calculation - remove redundant slice make - rename container metrics -> worker metrics - rename gatherLoop -> gatherMetricsEvery

dkarpukhin · 2018-01-16T16:21:39Z

isolate/initialdispatch.go

+			return nil, err
+		}
+
+		return d.onWorkersMetrics(uuids)


are u sure that not
return d.onWorkersMetrics(uuids), nil
?

I bet he's sure)

antmat

Rather big PR - still not fully reviewed.

antmat · 2018-01-17T14:23:40Z

isolate/initialdispatch.go

+
+	sz, err = r.ReadArrayHeader()
+	if err != nil {
+		return uuids, err


just return?

antmat · 2018-01-17T14:25:36Z

isolate/initialdispatch.go

+			return nil, err
+		}
+
+		return d.onWorkersMetrics(uuids)


I bet he's sure)

antmat · 2018-01-17T14:26:27Z

isolate/initialdispatch.go

+
+	startTime := time.Now()
+
+	sendMetricsFunc := func(metrics MetricsResponse) {


Maybe better to make as a standalone function or method?

antmat · 2018-01-17T14:31:30Z

isolate/initialdispatch.go

+		)
+
+		if d == nil {
+			log.G(d.ctx).Error("strange: dispatch is `nil`")


can it happen? If no - maybe remove, so we can get a crash when logic is being somehow violated?

antmat · 2018-01-17T14:38:36Z

isolate/porto/metrics_gatherer.go

+        select {
+        case <- ctx.Done():
+            return
+        case <-time.After(interval):


maybe it' better to use time.Ticker it should spawn less goroutines

codecov-io · 2018-01-25T12:25:15Z

Codecov Report

Merging #148 into master will decrease coverage by 7.04%.
The diff coverage is 0.25%.

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
- Coverage   35.98%   28.93%   -7.05%     
==========================================
  Files          41       45       +4     
  Lines        3185     3981     +796     
==========================================
+ Hits         1146     1152       +6     
- Misses       1758     2548     +790     
  Partials      281      281

Impacted Files	Coverage Δ
isolate/isolate.go	`14.28% <ø> (ø)`	⬆️
isolate/initialdispatch.go	`36.63% <0%> (-12.06%)`	⬇️
isolate/container_metrics.go	`0% <0%> (ø)`
isolate/conn_handler.go	`0% <0%> (ø)`	⬆️
isolate/process/metrics_gatherer.go	`0% <0%> (ø)`
isolate/docker/box.go	`71.82% <0%> (-0.81%)`	⬇️
isolate/container_metrics_gen.go	`0% <0%> (ø)`
isolate/porto/box.go	`0% <0%> (ø)`	⬆️
isolate/porto/metrics_gatherer.go	`0% <0%> (ø)`
isolate/process/box.go	`59.37% <5%> (-10.76%)`	⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2371b3e...e27c3b4. Read the comment docs.

karitra added 14 commits December 21, 2017 17:05

feat: few additional error types

e4a6b91

refact: Box/ResponseStream interface update

4c9565b

feat(conn_handler): WriteMessage for messagepack payload

236a170

feat(initialdispatch): metrics requests

4011378

feat(Porto): metrics collector

4abae05

feat: container metrics structs

815aed3

chore: update Makefile with msgp generator step

b458a1b

feat: add msgpack tags to metrics structs fields

30afd3d

+ show uuids metrics query in debug log

feat: metrics gatherer for legacy process subsys

a278f9d

feat(isolate): metrics poll config struct

a73df5d

refact: porto metrics collector

8966316

feat: metrics processing time in debug log

79e4d6c

feat: use fancy string period in config

1dcca85

refactor: bunch of updates

a8d4997

- (porto) cpu load calculation - remove redundant slice make - rename container metrics -> worker metrics - rename gatherLoop -> gatherMetricsEvery

karitra changed the title ~~Basci workers stats~~ Basic workers stats Jan 16, 2018

dkarpukhin reviewed Jan 16, 2018

View reviewed changes

chore: make gofmt a bit happier

0bb4c69

antmat reviewed Jan 17, 2018

View reviewed changes

karitra force-pushed the per_worker_stat branch 2 times, most recently from 4a493a3 to e19d3a7 Compare January 19, 2018 14:31

refact: post review changes

7632da4

karitra force-pushed the per_worker_stat branch from e19d3a7 to 7632da4 Compare January 19, 2018 14:51

karitra added 3 commits January 25, 2018 14:31

refact: get rid of go generate msgp

a1f5209

refact(porto): display less errors

cf5aa80

refact(process): drop gopsutil (it is fat)

a2e0be3

test: QueryMetrics, WriteMessage stubs

e9a0441

karitra force-pushed the per_worker_stat branch from c545557 to e9a0441 Compare January 25, 2018 15:54

refact: post pernicious ghost review updates

e27c3b4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic workers stats#148

Basic workers stats#148
karitra wants to merge 21 commits intointeriorem:masterfrom
karitra:per_worker_stat

karitra commented Jan 16, 2018

Uh oh!

dkarpukhin Jan 16, 2018

Uh oh!

antmat Jan 17, 2018

Uh oh!

antmat left a comment

Uh oh!

antmat Jan 17, 2018

Uh oh!

antmat Jan 17, 2018

Uh oh!

antmat Jan 17, 2018

Uh oh!

antmat Jan 17, 2018

Uh oh!

antmat Jan 17, 2018

Uh oh!

codecov-io commented Jan 25, 2018 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		startTime := time.Now()

		sendMetricsFunc := func(metrics MetricsResponse) {

Conversation

karitra commented Jan 16, 2018

Setup (via config file)

Uh oh!

dkarpukhin Jan 16, 2018

Choose a reason for hiding this comment

Uh oh!

antmat Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

antmat left a comment

Choose a reason for hiding this comment

Uh oh!

antmat Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

antmat Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

antmat Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

antmat Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

antmat Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jan 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Jan 25, 2018 •

edited

Loading