Skip to content

Commit 9d52bde

Browse files
dashpoleMrAliaspellared
authored
Use Set hash in Distinct (2nd attempt) (#7175)
Re-opening #5028 with new benchmarks. For cases with 10 attributes, this reduces the overhead of metric measurements by ~80-90% (depending on lock contention). It introduces a small probability of collision for attribute sets in the metrics SDK. For an instrument with 1 million different attribute sets, the probability of a collision is approximately 2 * 10^-8. For a more "normal" cardinality of 1000 on an instrument, it is approximately 2 * 10^-17. ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/attribute cpu: Intel(R) Xeon(R) CPU @ 2.20GHz │ main.txt │ hash.txt │ │ sec/op │ sec/op vs base │ EquivalentMapAccess/Empty-24 32.01n ± 2% 10.12n ± 4% -68.37% (p=0.002 n=6) EquivalentMapAccess/1_string_attribute-24 106.25n ± 2% 10.01n ± 5% -90.58% (p=0.002 n=6) EquivalentMapAccess/10_string_attributes-24 826.250n ± 1% 9.982n ± 11% -98.79% (p=0.002 n=6) EquivalentMapAccess/1_int_attribute-24 106.65n ± 2% 10.13n ± 3% -90.50% (p=0.002 n=6) EquivalentMapAccess/10_int_attributes-24 833.25n ± 2% 10.04n ± 5% -98.80% (p=0.002 n=6) geomean 190.3n 10.06n -94.72% ``` Parallel benchmarks: ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/metric cpu: Intel(R) Xeon(R) CPU @ 2.20GHz │ main24.txt │ new24.txt │ │ sec/op │ sec/op vs base │ SyncMeasure/NoView/Int64Counter/Attributes/0-24 288.4n ± 13% 267.0n ± 16% ~ (p=0.180 n=6) SyncMeasure/NoView/Int64Counter/Attributes/1-24 372.7n ± 24% 303.3n ± 6% -18.61% (p=0.002 n=6) SyncMeasure/NoView/Int64Counter/Attributes/10-24 1862.5n ± 11% 302.2n ± 6% -83.77% (p=0.002 n=6) SyncMeasure/NoView/Float64Counter/Attributes/0-24 288.2n ± 5% 291.8n ± 14% ~ (p=0.589 n=6) SyncMeasure/NoView/Float64Counter/Attributes/1-24 374.8n ± 22% 326.2n ± 15% -12.98% (p=0.002 n=6) SyncMeasure/NoView/Float64Counter/Attributes/10-24 1984.0n ± 10% 277.9n ± 15% -85.99% (p=0.002 n=6) SyncMeasure/NoView/Int64UpDownCounter/Attributes/0-24 286.8n ± 13% 279.4n ± 14% ~ (p=0.818 n=6) SyncMeasure/NoView/Int64UpDownCounter/Attributes/1-24 415.4n ± 14% 309.5n ± 11% -25.47% (p=0.002 n=6) SyncMeasure/NoView/Int64UpDownCounter/Attributes/10-24 1923.0n ± 19% 294.1n ± 17% -84.71% (p=0.002 n=6) SyncMeasure/NoView/Float64UpDownCounter/Attributes/0-24 284.9n ± 5% 271.6n ± 11% ~ (p=0.240 n=6) SyncMeasure/NoView/Float64UpDownCounter/Attributes/1-24 382.9n ± 23% 295.7n ± 13% -22.78% (p=0.002 n=6) SyncMeasure/NoView/Float64UpDownCounter/Attributes/10-24 1787.0n ± 28% 289.2n ± 12% -83.81% (p=0.002 n=6) SyncMeasure/NoView/Int64Histogram/Attributes/0-24 283.4n ± 8% 269.9n ± 9% ~ (p=0.589 n=6) SyncMeasure/NoView/Int64Histogram/Attributes/1-24 300.7n ± 8% 270.1n ± 15% -10.16% (p=0.026 n=6) SyncMeasure/NoView/Int64Histogram/Attributes/10-24 1046.8n ± 24% 299.2n ± 16% -71.42% (p=0.002 n=6) SyncMeasure/NoView/Float64Histogram/Attributes/0-24 264.3n ± 12% 295.9n ± 5% +11.93% (p=0.026 n=6) SyncMeasure/NoView/Float64Histogram/Attributes/1-24 321.0n ± 8% 269.4n ± 11% -16.09% (p=0.002 n=6) SyncMeasure/NoView/Float64Histogram/Attributes/10-24 1052.2n ± 10% 274.6n ± 5% -73.90% (p=0.002 n=6) geomean 540.0n 287.7n -46.72% ``` Single-threaded benchmarks: ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/metric cpu: Intel(R) Xeon(R) CPU @ 2.20GHz │ main1.txt │ new1.txt │ │ sec/op │ sec/op vs base │ SyncMeasure/NoView/Int64Counter/Attributes/0 130.95n ± 1% 97.99n ± 21% -25.17% (p=0.002 n=6) SyncMeasure/NoView/Int64Counter/Attributes/1 300.8n ± 7% 104.6n ± 3% -65.21% (p=0.002 n=6) SyncMeasure/NoView/Int64Counter/Attributes/10 1646.0n ± 2% 105.8n ± 2% -93.58% (p=0.002 n=6) SyncMeasure/NoView/Float64Counter/Attributes/0 132.65n ± 1% 99.28n ± 4% -25.16% (p=0.002 n=6) SyncMeasure/NoView/Float64Counter/Attributes/1 295.4n ± 3% 107.7n ± 3% -63.54% (p=0.002 n=6) SyncMeasure/NoView/Float64Counter/Attributes/10 1620.0n ± 1% 109.6n ± 4% -93.23% (p=0.002 n=6) SyncMeasure/NoView/Int64UpDownCounter/Attributes/0 132.85n ± 80% 99.34n ± 1% -25.22% (p=0.002 n=6) SyncMeasure/NoView/Int64UpDownCounter/Attributes/1 300.4n ± 1% 106.0n ± 1% -64.71% (p=0.002 n=6) SyncMeasure/NoView/Int64UpDownCounter/Attributes/10 1622.0n ± 1% 105.8n ± 1% -93.48% (p=0.002 n=6) SyncMeasure/NoView/Float64UpDownCounter/Attributes/0 134.90n ± 51% 99.16n ± 4% -26.49% (p=0.002 n=6) SyncMeasure/NoView/Float64UpDownCounter/Attributes/1 312.4n ± 34% 107.8n ± 2% -65.51% (p=0.002 n=6) SyncMeasure/NoView/Float64UpDownCounter/Attributes/10 1613.0n ± 23% 106.1n ± 1% -93.43% (p=0.002 n=6) SyncMeasure/NoView/Int64Histogram/Attributes/0 103.50n ± 17% 88.53n ± 1% -14.46% (p=0.002 n=6) SyncMeasure/NoView/Int64Histogram/Attributes/1 199.50n ± 16% 95.44n ± 2% -52.16% (p=0.002 n=6) SyncMeasure/NoView/Int64Histogram/Attributes/10 878.70n ± 2% 95.78n ± 2% -89.10% (p=0.002 n=6) SyncMeasure/NoView/Float64Histogram/Attributes/0 108.55n ± 54% 88.45n ± 1% -18.51% (p=0.002 n=6) SyncMeasure/NoView/Float64Histogram/Attributes/1 257.30n ± 14% 95.05n ± 2% -63.06% (p=0.002 n=6) SyncMeasure/NoView/Float64Histogram/Attributes/10 882.70n ± 18% 96.28n ± 1% -89.09% (p=0.002 n=6) geomean 355.2n 100.3n -71.77% ``` --------- Co-authored-by: Tyler Yahn <[email protected]> Co-authored-by: Robert Pająk <[email protected]>
1 parent 666f95c commit 9d52bde

File tree

7 files changed

+654
-54
lines changed

7 files changed

+654
-54
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1212

1313
- Add `WithInstrumentationAttributeSet` option to `go.opentelemetry.io/otel/log`, `go.opentelemetry.io/otel/metric`, and `go.opentelemetry.io/otel/trace` packages.
1414
This provides a concurrent-safe and performant alternative to `WithInstrumentationAttributes` by accepting a pre-constructed `attribute.Set`. (#7287)
15+
- Greatly reduce the cost of recording metrics in `go.opentelemetry.io/otel/sdk/metric` using hashing for map keys. (#7175)
1516

1617
### Fixed
1718

@@ -29,6 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2930
- `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/trace` synchronously de-duplicates the passed attributes instead of delegating it to the returned `TracerOption`. (#7266)
3031
- `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/meter` synchronously de-duplicates the passed attributes instead of delegating it to the returned `MeterOption`. (#7266)
3132
- `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/log` synchronously de-duplicates the passed attributes instead of delegating it to the returned `LoggerOption`. (#7266)
33+
- `Distinct` in `go.opentelemetry.io/otel/attribute` is no longer guaranteed to uniquely identify an attribute set. Collisions between `Distinct` values for different Sets are possible with extremely high cardinality (billions of series per instrument), but are highly unlikely. (#7175)
3234

3335
<!-- Released section -->
3436
<!-- Don't change this section unless doing release -->

Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,12 @@ build-tests/%:
146146

147147
# Tests
148148

149-
TEST_TARGETS := test-default test-bench test-short test-verbose test-race test-concurrent-safe
149+
TEST_TARGETS := test-default test-bench test-short test-verbose test-race test-concurrent-safe test-fuzz
150150
.PHONY: $(TEST_TARGETS) test
151151
test-default test-race: ARGS=-race
152152
test-bench: ARGS=-run=xxxxxMatchNothingxxxxx -test.benchtime=1ms -bench=.
153153
test-short: ARGS=-short
154+
test-fuzz: ARGS=-fuzztime=10s -fuzz
154155
test-verbose: ARGS=-v -race
155156
test-concurrent-safe: ARGS=-run=ConcurrentSafe -count=100 -race
156157
test-concurrent-safe: TIMEOUT=120

attribute/hash.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package attribute // import "go.opentelemetry.io/otel/attribute"
5+
6+
import (
7+
"fmt"
8+
"reflect"
9+
10+
"go.opentelemetry.io/otel/attribute/internal/fnv"
11+
)
12+
13+
// Type identifiers. These identifiers are hashed before the value of the
14+
// corresponding type. This is done to distinguish values that are hashed with
15+
// the same value representation (e.g. `int64(1)` and `true`, []int64{0} and
16+
// int64(0)).
17+
//
18+
// These are all 8 byte length strings converted to a uint64 representation. A
19+
// uint64 is used instead of the string directly as an optimization, it avoids
20+
// the for loop in [fnv] which adds minor overhead.
21+
const (
22+
boolID uint64 = 7953749933313450591 // "_boolean" (little endian)
23+
int64ID uint64 = 7592915492740740150 // "64_bit_i" (little endian)
24+
float64ID uint64 = 7376742710626956342 // "64_bit_f" (little endian)
25+
stringID uint64 = 6874584755375207263 // "_string_" (little endian)
26+
boolSliceID uint64 = 6875993255270243167 // "_[]bool_" (little endian)
27+
int64SliceID uint64 = 3762322556277578591 // "_[]int64" (little endian)
28+
float64SliceID uint64 = 7308324551835016539 // "[]double" (little endian)
29+
stringSliceID uint64 = 7453010373645655387 // "[]string" (little endian)
30+
)
31+
32+
// hashKVs returns a new FNV-1a hash of kvs.
33+
func hashKVs(kvs []KeyValue) fnv.Hash {
34+
h := fnv.New()
35+
for _, kv := range kvs {
36+
h = hashKV(h, kv)
37+
}
38+
return h
39+
}
40+
41+
// hashKV returns the FNV-1a hash of kv with h as the base.
42+
func hashKV(h fnv.Hash, kv KeyValue) fnv.Hash {
43+
h = h.String(string(kv.Key))
44+
45+
switch kv.Value.Type() {
46+
case BOOL:
47+
h = h.Uint64(boolID)
48+
h = h.Uint64(kv.Value.numeric)
49+
case INT64:
50+
h = h.Uint64(int64ID)
51+
h = h.Uint64(kv.Value.numeric)
52+
case FLOAT64:
53+
h = h.Uint64(float64ID)
54+
// Assumes numeric stored with math.Float64bits.
55+
h = h.Uint64(kv.Value.numeric)
56+
case STRING:
57+
h = h.Uint64(stringID)
58+
h = h.String(kv.Value.stringly)
59+
case BOOLSLICE:
60+
h = h.Uint64(boolSliceID)
61+
rv := reflect.ValueOf(kv.Value.slice)
62+
for i := 0; i < rv.Len(); i++ {
63+
h = h.Bool(rv.Index(i).Bool())
64+
}
65+
case INT64SLICE:
66+
h = h.Uint64(int64SliceID)
67+
rv := reflect.ValueOf(kv.Value.slice)
68+
for i := 0; i < rv.Len(); i++ {
69+
h = h.Int64(rv.Index(i).Int())
70+
}
71+
case FLOAT64SLICE:
72+
h = h.Uint64(float64SliceID)
73+
rv := reflect.ValueOf(kv.Value.slice)
74+
for i := 0; i < rv.Len(); i++ {
75+
h = h.Float64(rv.Index(i).Float())
76+
}
77+
case STRINGSLICE:
78+
h = h.Uint64(stringSliceID)
79+
rv := reflect.ValueOf(kv.Value.slice)
80+
for i := 0; i < rv.Len(); i++ {
81+
h = h.String(rv.Index(i).String())
82+
}
83+
case INVALID:
84+
default:
85+
// Logging is an alternative, but using the internal logger here
86+
// causes an import cycle so it is not done.
87+
v := kv.Value.AsInterface()
88+
msg := fmt.Sprintf("unknown value type: %[1]v (%[1]T)", v)
89+
panic(msg)
90+
}
91+
return h
92+
}

0 commit comments

Comments
 (0)