Add carbon parser #1309

richardartoul · 2019-01-19T21:37:00Z

Most of the code is copy-pasta from out statsdex repo, I just removed some old migration cruft and cleaned things up a tad.

codecov · 2019-01-19T21:46:17Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:46:21Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:46:27Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:46:35Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:46:44Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:46:54Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:46:55Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:46:59Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.5%.
The diff coverage is 10.7%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.3%   -0.6%     
========================================
  Files         769     771      +2     
  Lines       64590   64702    +112     
========================================
- Hits        45789   45493    -296     
- Misses      15839   16269    +430     
+ Partials     2962    2940     -22

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.9% <0%> (-0.3%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`76.2% <100%> (+0.3%)`	⬆️

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 65fadfe...66f6c62. Read the comment docs.

codecov · 2019-01-19T21:47:02Z

Codecov Report

Merging #1309 into master will decrease coverage by 0.1%.
The diff coverage is 0%.

@@           Coverage Diff            @@
##           master   #1309     +/-   ##
========================================
- Coverage    70.8%   70.7%   -0.2%     
========================================
  Files         769     770      +1     
  Lines       64590   64714    +124     
========================================
- Hits        45789   45787      -2     
- Misses      15839   15963    +124     
- Partials     2962    2964      +2

Flag	Coverage Δ
#aggregator	`80.9% <ø> (ø)`	⬆️
#cluster	`85.6% <ø> (ø)`	⬆️
#collector	`78.4% <ø> (ø)`	⬆️
#dbnode	`80.9% <ø> (-0.1%)`	⬇️
#m3em	`73.2% <ø> (ø)`	⬆️
#m3ninx	`74.6% <ø> (ø)`	⬆️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.8% <0%> (-0.4%)`	⬇️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.9% <ø> (ø)`	⬆️
#x	`75.9% <ø> (ø)`	⬆️

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 65fadfe...2c38b8b. Read the comment docs.

robskillington · 2019-01-21T02:03:21Z

src/metrics/carbon/parser.go

+// ParsePacket parses a carbon packet and returns the metrics and number of malformed lines.
+func ParsePacket(packet []byte) ([]Metric, int) {
+	var malformed, prevIdx, i int
+	mets := []Metric{}


Maybe make this a param to pass in? So we can pool the slices.

Can split up into two methods to make both the nice and the efficient APIs available:

func ParsePacket(packet []byte) ([]Metric, int) {} func AppendAndParsePacket(slice []Metric, packet []byte) ([]Metric, int) {}

src/metrics/carbon/parser_benchmark_test.go

src/metrics/carbon/parser.go

robskillington · 2019-01-21T02:09:18Z

src/metrics/carbon/parser.go

+	timestamp time.Time
+	path      []byte
+	value     float64
+	recv      time.Time


Can remove recv, not used.

src/metrics/carbon/parser_test.go

robskillington · 2019-01-21T18:34:25Z

src/metrics/carbon/parser.go

+		if val := strings.ToLower(s[valStart:valEnd]); val == negativeNanStr || val == nanStr {
+			value = mathNan
+		} else {
+			value, err = strconv.ParseFloat(s[valStart:valEnd], 64)


nit: Replace 64 with floatBitSize?

robskillington · 2019-01-21T18:35:03Z

src/metrics/carbon/parser.go

+	// allocating a string.
+	var tsInSecs int64
+	unsafe.WithString(rest, func(s string) {
+		tsInSecs, err = strconv.ParseInt(s[secStart:secEnd], 10, 64)


nit: Add a const intBitSize = 64 and use the existing intBase?

robskillington

LGTM other than nits

Richard Artoul added 2 commits January 19, 2019 16:19

copy over carbon package without migration code

fdb8385

dont use statsdex code and cleanup

66f6c62

Richard Artoul added 5 commits January 20, 2019 14:42

update deps

7dae12e

working

96ffa8e

refactor and cleanup

f28d2ba

make remainder not require name

57654ad

Clarify comment

3c9b07d