🚀 Speed up toMetricTagsID

[bmoylan]

toMetricTagsID is called every time a metric is incremented/changed. This PR updates the implementation to run faster and allocate less memory.

Benchmark results:
Old

goos: darwin
goarch: amd64
pkg: github.com/palantir/pkg/metrics
BenchmarkRegisterMetric/1_tag-8          2000000     908 ns/op     800 B/op   10 allocs/op
BenchmarkRegisterMetric/10_tag-8          300000    4905 ns/op    2910 B/op   27 allocs/op
BenchmarkRegisterMetric/100_tag-8          30000   53716 ns/op   30639 B/op  136 allocs/op
PASS
ok  	github.com/palantir/pkg/metrics	6.802s

New

goos: darwin
goarch: amd64
pkg: github.com/palantir/pkg/metrics
BenchmarkRegisterMetric/1_tag-8          5000000     300 ns/op     208 B/op    4 allocs/op
BenchmarkRegisterMetric/10_tag-8         2000000     657 ns/op     368 B/op    4 allocs/op
BenchmarkRegisterMetric/100_tag-8         200000    8304 ns/op    2704 B/op    4 allocs/op
PASS
ok  	github.com/palantir/pkg/metrics	5.880s

This was flagged by an internal team looking at the pprofs of their service.

---

This change is 

[nmiyake]

Awesome, thanks for the PR! Minor suggestion on creating a private new function for struct initialization, but otherwise looks good. Hopefully this improvement is good enough, but happy to brainstorm if it looks like there may be deeper structural issues that may need to be addressed.

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @bmoylan)

---

metrics/tag.go, line 110 at r1 (raw file):

		value: normalizeTag(v),
	}
	tag.keyValue = tag.key + ":" + tag.value

Might be worth creating a newTag(k, v) function -- that function can handle normalization of k and v and ensuring keyValue is set.

[bmoylan]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @nmiyake)

---

metrics/tag.go, line 110 at r1 (raw file):

Previously, nmiyake (Nick Miyake) wrote…

Might be worth creating a newTag(k, v) function -- that function can handle normalization of k and v and ensuring keyValue is set.

Done.

[nmiyake]

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @nmiyake and @bmoylan)

---

metrics/tag.go, line 109 at r2 (raw file):

}

func newTag(normalizedKey, normakizedValue string) Tag {

Actually I think it'd be nicer if this just accepted the non-normalized values and we performed the normalization as part of the function.

There's also a typo in normakizedValue, but probably won't matter as if the suggestion is followed the variable names should be updated as well

[bmoylan]

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @nmiyake)

---

metrics/tag.go, line 109 at r2 (raw file):

Previously, nmiyake (Nick Miyake) wrote…

Actually I think it'd be nicer if this just accepted the non-normalized values and we performed the normalization as part of the function.

There's also a typo in normakizedValue, but probably won't matter as if the suggestion is followed the variable names should be updated as well

ack, fixed!

[nmiyake]

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved