Add map<string, AnyValue> to the attribute package#7943
Add map<string, AnyValue> to the attribute package#7943itssaharsh wants to merge 20 commits intoopen-telemetry:mainfrom
attribute package#7943Conversation
|
Changing to draft as this does not look to be ready for review. |
|
Okay this was an initial work overall and I will use the references now and change accordingly |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7943 +/- ##
=====================================
Coverage 81.6% 81.7%
=====================================
Files 304 304
Lines 23424 23510 +86
=====================================
+ Hits 19127 19211 +84
- Misses 3909 3910 +1
- Partials 388 389 +1
🚀 New features to boost your workflow:
|
|
@itssaharsh, are you able to work on it this week? MAP is the only type that we do not have a PR "ready". |
I think it is ready for review. I viewed the other PRs and changed my code accordingly |
I kindly ask you do it once more with more attention. |
attribute/value.go
Outdated
| // without type wrappers). | ||
| func (v Value) mapEmitValue() any { | ||
| m := v.asMap() | ||
| raw := make(map[string]any, len(m)) |
There was a problem hiding this comment.
This looks very inefficient as it would cause a lot of allocations. Can we try doing something similar to
opentelemetry-go/attribute/value.go
Lines 302 to 322 in dedc8f7
Note that Emit is currently used by the Prometheus exporter (in getAttrs function) so it has to be efficient.
| case math.IsInf(f, -1): | ||
| _, _ = b.WriteString(`"-Infinity"`) | ||
| default: | ||
| _, _ = b.WriteString(kv.Value.Emit()) |
There was a problem hiding this comment.
What would be more efficient Emit or json.Marshal?
Or maybe we should even use https://pkg.go.dev/encoding/json#NewEncoder?
There was a problem hiding this comment.
I think emit and marshal are around same benchmarks but encoder has less allocations.
https://github.com/bytedance/sonic found this it was shown that it is very fast but haven't used yet. Your thoughts?
There was a problem hiding this comment.
I think emit and marshal are around same benchmarks but encoder has less allocations
Leaning towards encoder but we should have benchmark to validate it. I also plan to improve the Emit in my PR for SLICE. I just need to find some time for it
https://github.com/bytedance/sonic found this it was shown that it is very fast but haven't used yet. Your thoughts?
We do not want to add any new dependency for this.
There was a problem hiding this comment.
Okay got it and I performed benchamrks locally and found that newencoder was around 10 percent slower and had around 7 percent more alocations
So should I use it and update the other json.Marshals also? and should I also post the benchmarks here
There was a problem hiding this comment.
So should I use it and updta the other json.Marshals also?
Yes, given it makes it more efficient.
and should I also post the benchmarks here
It would be very good to have even for sake of retrospecting.
There was a problem hiding this comment.
I added a extra function for the slices to be encoded to remove redudant code is that okay?
There was a problem hiding this comment.
The tests are too long I used
go test -bench=. -benchmem -count=5 -benchtime=5s -timeout=30m ./attribute/benchmark_test.go so the outputs are too long. Apologies for that should I add all the benchmarks here?
There was a problem hiding this comment.
No need to add these parameters -count=5 -benchtime=5s -timeout=30m
Co-authored-by: Robert Pająk <pellared@hotmail.com>
|
I donot understand why the links(fail fast) is failing, I need some help with that one |
|
Marshal is faster and better allocation wise |
|
@itssaharsh use I thought you just wanted to point out the "current state". |
I see the opposite. |
I saw that Marshal take more time overall and indivisually also and allocs also letme use benchstat and upload them here sorry for the confusion |
|
I think that marshal is better somewhat so should I revert back to marshal? I think marshal also escapes &,<,> so should I specify these in case string also? |
|
Please double check the output (with understanding 😉 )
Look that encoder for instance here is probably better
For map type the encoder is reused multiple types. |
│ marshal.txt │ encoder.txt │
│ sec/op │ sec/op vs base │
Bool/Value-16 2.255n ± 8% 2.426n ± 6% +7.58% (p=0.002 n=7)
Bool/KeyValue-16 22.39n ± 1% 24.51n ± 3% +9.47% (p=0.001 n=7)
Bool/AsBool-16 2.221n ± 1% 2.438n ± 6% +9.77% (p=0.001 n=7)
Bool/Emit-16 24.64n ± 1% 27.97n ± 5% +13.51% (p=0.001 n=7)
BoolSlice/Value-16 208.7n ± 1% 227.0n ± 3% +8.77% (p=0.001 n=7)
BoolSlice/KeyValue-16 210.4n ± 8% 226.2n ± 7% ~ (p=0.072 n=7)
BoolSlice/AsBoolSlice-16 84.56n ± 1% 86.47n ± 6% ~ (p=0.710 n=7)
BoolSlice/Emit-16 769.5n ± 1% 771.4n ± 4% ~ (p=0.318 n=7)
Int/Value-16 2.222n ± 8% 2.436n ± 6% +9.63% (p=0.004 n=7)
Int/KeyValue-16 24.08n ± 0% 24.35n ± 7% ~ (p=0.710 n=7)
Int/Emit-16 29.28n ± 1% 29.43n ± 7% ~ (p=0.902 n=7)
IntSlice/Value-16 251.8n ± 4% 260.8n ± 10% ~ (p=0.259 n=7)
IntSlice/KeyValue-16 269.2n ± 1% 273.4n ± 7% +1.56% (p=0.038 n=7)
IntSlice/Emit-16 594.4n ± 6% 716.9n ± 6% +20.61% (p=0.001 n=7)
Int64/Value-16 2.289n ± 2% 2.463n ± 7% +7.60% (p=0.001 n=7)
Int64/KeyValue-16 23.60n ± 2% 24.13n ± 6% ~ (p=0.195 n=7)
Int64/AsInt64-16 2.374n ± 4% 2.403n ± 5% ~ (p=0.165 n=7)
Int64/Emit-16 28.81n ± 6% 29.74n ± 7% ~ (p=0.259 n=7)
Int64Slice/Value-16 243.5n ± 4% 245.3n ± 7% ~ (p=0.097 n=7)
Int64Slice/KeyValue-16 255.0n ± 5% 253.9n ± 3% ~ (p=0.620 n=7)
Int64Slice/AsInt64Slice-16 93.03n ± 2% 99.70n ± 5% +7.17% (p=0.001 n=7)
Int64Slice/Emit-16 575.5n ± 3% 716.8n ± 6% +24.55% (p=0.001 n=7)
Float64/Value-16 2.397n ± 4% 2.442n ± 6% ~ (p=0.165 n=7)
Float64/KeyValue-16 23.21n ± 0% 24.32n ± 2% +4.78% (p=0.001 n=7)
Float64/AsFloat64-16 2.368n ± 2% 2.401n ± 6% +1.39% (p=0.034 n=7)
Float64/Emit-16 251.2n ± 5% 259.0n ± 4% ~ (p=0.778 n=7)
Float64Slice/Value-16 247.9n ± 3% 254.1n ± 3% +2.50% (p=0.004 n=7)
Float64Slice/KeyValue-16 248.7n ± 1% 260.3n ± 5% +4.66% (p=0.038 n=7)
Float64Slice/AsFloat64Slice-16 93.38n ± 1% 98.42n ± 6% +5.40% (p=0.026 n=7)
Float64Slice/Emit-16 933.4n ± 6% 1068.0n ± 2% +14.42% (p=0.001 n=7)
String/Value-16 3.080n ± 2% 3.238n ± 4% +5.13% (p=0.010 n=7)
String/KeyValue-16 22.36n ± 3% 22.74n ± 7% ~ (p=0.097 n=7)
String/AsString-16 2.861n ± 1% 2.749n ± 7% -3.91% (p=0.024 n=7)
String/Emit-16 24.53n ± 3% 25.96n ± 4% +5.83% (p=0.001 n=7)
StringSlice/Value-16 288.8n ± 2% 308.6n ± 3% +6.86% (p=0.001 n=7)
StringSlice/KeyValue-16 300.3n ± 4% 293.6n ± 8% ~ (p=0.973 n=7)
StringSlice/AsStringSlice-16 123.7n ± 1% 131.7n ± 5% +6.47% (p=0.002 n=7)
StringSlice/Emit-16 687.5n ± 3% 825.4n ± 6% +20.06% (p=0.001 n=7)
Map/Value-16 1.352µ ± 2% 1.368µ ± 10% ~ (p=0.105 n=7)
Map/KeyValue-16 1.328µ ± 2% 1.268µ ± 10% ~ (p=0.259 n=7)
Map/AsMap-16 834.5n ± 1% 769.9n ± 8% -7.74% (p=0.004 n=7)
Map/Emit-16 3.620µ ± 2% 3.646µ ± 3% ~ (p=0.057 n=7)
SetEquals/Empty-16 19.03n ± 3% 19.57n ± 1% +2.84% (p=0.001 n=7)
SetEquals/1_string_attribute-16 38.21n ± 3% 37.09n ± 4% -2.93% (p=0.006 n=7)
SetEquals/10_string_attributes-16 191.1n ± 1% 180.9n ± 1% -5.34% (p=0.001 n=7)
SetEquals/1_int_attribute-16 38.51n ± 1% 37.23n ± 1% -3.32% (p=0.001 n=7)
SetEquals/10_int_attributes-16 187.1n ± 11% 185.2n ± 3% ~ (p=0.710 n=7)
EquivalentMapAccess/Empty-16 14.49n ± 2% 15.38n ± 4% +6.14% (p=0.001 n=7)
EquivalentMapAccess/1_string_attribute-16 14.58n ± 9% 15.53n ± 2% ~ (p=0.118 n=7)
EquivalentMapAccess/10_string_attributes-16 15.97n ± 7% 15.41n ± 3% -3.51% (p=0.002 n=7)
EquivalentMapAccess/1_int_attribute-16 16.92n ± 1% 15.05n ± 2% -11.05% (p=0.001 n=7)
EquivalentMapAccess/10_int_attributes-16 16.98n ± 1% 14.84n ± 2% -12.60% (p=0.001 n=7)
geomean 59.59n 61.58n +3.34%
│ marshal.txt │ encoder.txt │
│ B/op │ B/op vs base │
Bool/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Bool/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Bool/AsBool-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Bool/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/Value-16 6.000 ± 0% 6.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/KeyValue-16 6.000 ± 0% 6.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/AsBoolSlice-16 3.000 ± 0% 3.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/Emit-16 54.00 ± 0% 54.00 ± 0% ~ (p=1.000 n=7) ¹
Int/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
IntSlice/Value-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=7) ¹
IntSlice/KeyValue-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=7) ¹
IntSlice/Emit-16 80.00 ± 0% 176.00 ± 0% +120.00% (p=0.001 n=7)
Int64/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64/AsInt64-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/Value-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/KeyValue-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/AsInt64Slice-16 24.00 ± 0% 24.00 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/Emit-16 80.00 ± 0% 176.00 ± 0% +120.00% (p=0.001 n=7)
Float64/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Float64/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Float64/AsFloat64-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Float64/Emit-16 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/Value-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/KeyValue-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/AsFloat64Slice-16 24.00 ± 0% 24.00 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/Emit-16 80.00 ± 0% 176.00 ± 0% +120.00% (p=0.001 n=7)
String/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
String/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
String/AsString-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
String/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/Value-16 96.00 ± 0% 96.00 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/KeyValue-16 96.00 ± 0% 96.00 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/AsStringSlice-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/Emit-16 168.0 ± 0% 232.0 ± 0% +38.10% (p=0.001 n=7)
Map/Value-16 1.016Ki ± 0% 1.016Ki ± 0% ~ (p=1.000 n=7) ¹
Map/KeyValue-16 1.016Ki ± 0% 1.016Ki ± 0% ~ (p=1.000 n=7) ¹
Map/AsMap-16 944.0 ± 0% 944.0 ± 0% ~ (p=1.000 n=7) ¹
Map/Emit-16 841.0 ± 0% 961.0 ± 0% +14.27% (p=0.001 n=7)
SetEquals/Empty-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/1_string_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/10_string_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/1_int_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/10_int_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/Empty-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/1_string_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/10_string_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/1_int_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/10_int_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
geomean ² +5.58% ²
¹ all samples are equal
² summaries must be >0 to compute geomean
│ marshal.txt │ encoder.txt │
│ allocs/op │ allocs/op vs base │
Bool/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Bool/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Bool/AsBool-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Bool/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/Value-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/KeyValue-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/AsBoolSlice-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=7) ¹
BoolSlice/Emit-16 6.000 ± 0% 6.000 ± 0% ~ (p=1.000 n=7) ¹
Int/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
IntSlice/Value-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
IntSlice/KeyValue-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
IntSlice/Emit-16 4.000 ± 0% 5.000 ± 0% +25.00% (p=0.001 n=7)
Int64/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64/AsInt64-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/Value-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/KeyValue-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/AsInt64Slice-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=7) ¹
Int64Slice/Emit-16 4.000 ± 0% 5.000 ± 0% +25.00% (p=0.001 n=7)
Float64/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Float64/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Float64/AsFloat64-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
Float64/Emit-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/Value-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/KeyValue-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/AsFloat64Slice-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=7) ¹
Float64Slice/Emit-16 4.000 ± 0% 5.000 ± 0% +25.00% (p=0.001 n=7)
String/Value-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
String/KeyValue-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
String/AsString-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
String/Emit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/Value-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/KeyValue-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/AsStringSlice-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=7) ¹
StringSlice/Emit-16 4.000 ± 0% 5.000 ± 0% +25.00% (p=0.001 n=7)
Map/Value-16 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=7) ¹
Map/KeyValue-16 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=7) ¹
Map/AsMap-16 3.000 ± 0% 3.000 ± 0% ~ (p=1.000 n=7) ¹
Map/Emit-16 24.00 ± 0% 23.00 ± 0% -4.17% (p=0.001 n=7)
SetEquals/Empty-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/1_string_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/10_string_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/1_int_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
SetEquals/10_int_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/Empty-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/1_string_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/10_string_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/1_int_attribute-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
EquivalentMapAccess/10_int_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=7) ¹
geomean ² +1.65% ²
¹ all samples are equal
² summaries must be >0 to compute geomean |
|
I read the benchmarks and I think that Encoder is slightly better in case of map and in other cases marshal is better or they are even. So let's use encoder for map and marshal for everything else. |
This PR focuses on adding the Support map<string, AnyValue> to
attributepackagetracked in: #7935