Skip to content

observability: constant time implementation of prometheusFieldName

Administrator requested to merge core/faster-prom-whitelist into master

Created by: keegancsmith

The list of fields is quite large so iterating over them has a cost. Luckily there is a simple change which changes the function from O(n) to O(1), use a map. The worse case is actually pretty common, since it will happen for any field which isn't whitelisted.

The downside of this is we may have annoying whitespace changes. We can introduce a helper function which takes a list of pairs and returns the map. We can do that if this becomes a problem. Alternatively we can introduce a fake entry which sufficiently pads the list to avoid gofmt being too helpful.

Thank you for adding a benchmark, it made it far too tempting to improve the implementation. Additionally I added a benchmark for the worst case (a miss). This commit also tests for correctness.

  name                          old time/op  new time/op  delta
  PrometheusFieldName/test-0-8   232ns ± 1%    29ns ± 1%  -87.31%  (p=0.000 n=9+10)
  PrometheusFieldName/test-1-8   260ns ± 2%    35ns ± 1%  -86.58%  (p=0.000 n=9+9)
  PrometheusFieldName/test-2-8   382ns ± 3%    33ns ± 1%  -91.26%  (p=0.000 n=10+10)
  PrometheusFieldName/test-3-8   366ns ± 6%    26ns ± 0%  -92.81%  (p=0.000 n=9+10)

Merge request reports