Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace recordingSpan attributes implementation with slice of attributes #2576

Merged
merged 15 commits into from Feb 7, 2022

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Feb 1, 2022

This is an alternate implementation to #2555. It is preferred to that PR because it realizes similar gains but maintains the order of attributes (in the order the user added them).

Instead of an LRU strategy for cap-ing span attributes, comply with the specification and drop last added. To do this, the attributesmap is replaced with a slice of attributes.

Testing performance comparison

This looks to have considerable memory and allocation reduction and faster computations when compared to main.

$ benchstat bench-main-53ead308 bench-recordingSpan-attrs-slice-326b6462

name                                          old time/op    new time/op    delta
SpanProcessor-8                                 14.9µs ±10%    11.3µs ±11%  -24.29%  (p=0.000 n=8+10)
SpanProcessorVerboseLogging-8                   15.5µs ± 3%    12.1µs ± 6%  -22.01%  (p=0.000 n=8+9)
StartEndSpan/AlwaysSample-8                      978ns ±14%     636ns ± 5%  -35.00%  (p=0.000 n=10+10)
StartEndSpan/NeverSample-8                       367ns ± 2%     313ns ± 2%  -14.96%  (p=0.000 n=8+9)
SpanSetAttributesOverCapacity-8                 23.1µs ± 5%     2.2µs ± 3%  -90.29%  (p=0.000 n=10+9)
SpanWithAttributes_4/AlwaysSample-8             1.85µs ± 5%    1.18µs ± 3%  -36.22%  (p=0.000 n=8+9)
SpanWithAttributes_4/NeverSample-8               513ns ± 7%     468ns ± 1%   -8.94%  (p=0.000 n=9+7)
SpanWithAttributes_8/AlwaysSample-8             2.68µs ± 5%    1.62µs ± 4%  -39.66%  (p=0.000 n=8+9)
SpanWithAttributes_8/NeverSample-8               679ns ± 5%     620ns ± 3%   -8.65%  (p=0.000 n=8+9)
SpanWithAttributes_all/AlwaysSample-8           2.09µs ± 7%    1.48µs ± 3%  -29.13%  (p=0.000 n=9+9)
SpanWithAttributes_all/NeverSample-8             566ns ± 5%     507ns ± 6%  -10.57%  (p=0.000 n=8+10)
SpanWithAttributes_all_2x/AlwaysSample-8        3.72µs ±12%    2.11µs ± 4%  -43.20%  (p=0.000 n=10+10)
SpanWithAttributes_all_2x/NeverSample-8          753ns ± 6%     692ns ± 6%   -8.15%  (p=0.000 n=8+10)
SpanWithEvents_4/AlwaysSample-8                 2.04µs ± 7%    1.50µs ± 2%  -26.61%  (p=0.000 n=10+7)
SpanWithEvents_4/NeverSample-8                   386ns ± 3%     322ns ± 3%  -16.46%  (p=0.000 n=9+10)
SpanWithEvents_8/AlwaysSample-8                 3.02µs ± 3%    2.27µs ± 1%  -24.75%  (p=0.000 n=9+8)
SpanWithEvents_8/NeverSample-8                   395ns ± 8%     333ns ± 3%  -15.68%  (p=0.000 n=10+10)
SpanWithEvents_WithStackTrace/AlwaysSample-8    1.26µs ± 8%    0.91µs ± 2%  -28.15%  (p=0.000 n=10+10)
SpanWithEvents_WithStackTrace/NeverSample-8      423ns ±10%     363ns ± 8%  -14.03%  (p=0.000 n=9+10)
SpanWithEvents_WithTimestamp/AlwaysSample-8     1.24µs ± 5%    0.92µs ± 2%  -25.81%  (p=0.000 n=10+8)
SpanWithEvents_WithTimestamp/NeverSample-8       470ns ± 2%     408ns ± 1%  -13.17%  (p=0.000 n=10+9)
TraceID_DotString-8                             82.2ns ± 5%    81.9ns ± 2%     ~     (p=0.897 n=10+8)
SpanID_DotString-8                              62.3ns ± 2%    61.5ns ± 2%     ~     (p=0.052 n=10+10)

name                                          old alloc/op   new alloc/op   delta
SpanProcessor-8                                 12.3kB ± 0%     8.4kB ± 0%  -31.29%  (p=0.000 n=10+10)
SpanProcessorVerboseLogging-8                   13.4kB ± 0%     9.6kB ± 0%  -28.62%  (p=0.000 n=10+9)
StartEndSpan/AlwaysSample-8                       816B ± 0%      432B ± 0%  -47.06%  (p=0.000 n=10+10)
StartEndSpan/NeverSample-8                        224B ± 0%      128B ± 0%  -42.86%  (p=0.000 n=10+10)
SpanSetAttributesOverCapacity-8                 15.4kB ± 0%     0.5kB ± 0%  -96.77%  (p=0.000 n=10+10)
SpanWithAttributes_4/AlwaysSample-8             1.73kB ± 0%    1.14kB ± 0%  -34.26%  (p=0.000 n=10+10)
SpanWithAttributes_4/NeverSample-8                480B ± 0%      384B ± 0%  -20.00%  (p=0.000 n=10+10)
SpanWithAttributes_8/AlwaysSample-8             2.43kB ± 0%    1.90kB ± 0%  -21.71%  (p=0.000 n=10+10)
SpanWithAttributes_8/NeverSample-8                736B ± 0%      640B ± 0%  -13.04%  (p=0.000 n=10+10)
SpanWithAttributes_all/AlwaysSample-8           1.90kB ± 0%    1.71kB ± 0%  -10.08%  (p=0.000 n=10+10)
SpanWithAttributes_all/NeverSample-8              544B ± 0%      448B ± 0%  -17.65%  (p=0.000 n=10+10)
SpanWithAttributes_all_2x/AlwaysSample-8        3.20kB ± 0%    3.06kB ± 0%   -4.62%  (p=0.000 n=10+10)
SpanWithAttributes_all_2x/NeverSample-8           864B ± 0%      768B ± 0%  -11.11%  (p=0.000 n=10+10)
SpanWithEvents_4/AlwaysSample-8                 1.50kB ± 0%    0.86kB ± 0%  -42.55%  (p=0.000 n=10+10)
SpanWithEvents_4/NeverSample-8                    224B ± 0%      128B ± 0%  -42.86%  (p=0.000 n=10+10)
SpanWithEvents_8/AlwaysSample-8                 2.21kB ± 0%    1.31kB ± 0%  -40.58%  (p=0.000 n=10+10)
SpanWithEvents_8/NeverSample-8                    224B ± 0%      128B ± 0%  -42.86%  (p=0.000 n=10+10)
SpanWithEvents_WithStackTrace/AlwaysSample-8      992B ± 0%      544B ± 0%  -45.16%  (p=0.000 n=10+10)
SpanWithEvents_WithStackTrace/NeverSample-8       240B ± 0%      144B ± 0%  -40.00%  (p=0.000 n=10+10)
SpanWithEvents_WithTimestamp/AlwaysSample-8     1.02kB ± 0%    0.57kB ± 0%  -44.09%  (p=0.000 n=10+10)
SpanWithEvents_WithTimestamp/NeverSample-8        264B ± 0%      168B ± 0%  -36.36%  (p=0.000 n=10+10)

name                                          old allocs/op  new allocs/op  delta
SpanProcessor-8                                   85.0 ± 0%      35.0 ± 0%  -58.82%  (p=0.000 n=10+10)
SpanProcessorVerboseLogging-8                     91.0 ± 0%      41.0 ± 0%  -54.95%  (p=0.000 n=10+10)
StartEndSpan/AlwaysSample-8                       7.00 ± 0%      2.00 ± 0%  -71.43%  (p=0.000 n=10+10)
StartEndSpan/NeverSample-8                        3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=10+10)
SpanSetAttributesOverCapacity-8                    264 ± 0%         3 ± 0%  -98.86%  (p=0.000 n=10+10)
SpanWithAttributes_4/AlwaysSample-8               17.0 ± 0%       6.0 ± 0%  -64.71%  (p=0.000 n=10+10)
SpanWithAttributes_4/NeverSample-8                4.00 ± 0%      3.00 ± 0%  -25.00%  (p=0.000 n=10+10)
SpanWithAttributes_8/AlwaysSample-8               25.0 ± 0%       7.0 ± 0%  -72.00%  (p=0.000 n=10+10)
SpanWithAttributes_8/NeverSample-8                4.00 ± 0%      3.00 ± 0%  -25.00%  (p=0.000 n=10+10)
SpanWithAttributes_all/AlwaysSample-8             19.0 ± 0%       7.0 ± 0%  -63.16%  (p=0.000 n=10+10)
SpanWithAttributes_all/NeverSample-8              4.00 ± 0%      3.00 ± 0%  -25.00%  (p=0.000 n=10+10)
SpanWithAttributes_all_2x/AlwaysSample-8          30.0 ± 0%       8.0 ± 0%  -73.33%  (p=0.000 n=10+10)
SpanWithAttributes_all_2x/NeverSample-8           4.00 ± 0%      3.00 ± 0%  -25.00%  (p=0.000 n=10+10)
SpanWithEvents_4/AlwaysSample-8                   18.0 ± 0%       9.0 ± 0%  -50.00%  (p=0.000 n=10+10)
SpanWithEvents_4/NeverSample-8                    3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=10+10)
SpanWithEvents_8/AlwaysSample-8                   27.0 ± 0%      14.0 ± 0%  -48.15%  (p=0.000 n=10+10)
SpanWithEvents_8/NeverSample-8                    3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=10+10)
SpanWithEvents_WithStackTrace/AlwaysSample-8      11.0 ± 0%       5.0 ± 0%  -54.55%  (p=0.000 n=10+10)
SpanWithEvents_WithStackTrace/NeverSample-8       4.00 ± 0%      3.00 ± 0%  -25.00%  (p=0.000 n=10+10)
SpanWithEvents_WithTimestamp/AlwaysSample-8       12.0 ± 0%       6.0 ± 0%  -50.00%  (p=0.000 n=10+10)
SpanWithEvents_WithTimestamp/NeverSample-8        5.00 ± 0%      4.00 ± 0%  -20.00%  (p=0.000 n=10+10)

MrAlias and others added 2 commits February 1, 2022 15:49
Instead of an LRU strategy for cap-ing span attributes, comply with the
specification and drop last added. To do this, the attributesmap is
replaced with a slice of attributes.
@codecov
Copy link

codecov bot commented Feb 2, 2022

Codecov Report

Merging #2576 (9ae80f7) into main (b60d53d) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2576     +/-   ##
=======================================
+ Coverage   76.0%   76.2%   +0.1%     
=======================================
  Files        174     173      -1     
  Lines      12214   12229     +15     
=======================================
+ Hits        9292    9319     +27     
+ Misses      2677    2667     -10     
+ Partials     245     243      -2     
Impacted Files Coverage Δ
sdk/trace/span.go 86.4% <100.0%> (+4.3%) ⬆️
sdk/trace/tracer.go 100.0% <100.0%> (ø)
exporters/jaeger/jaeger.go 93.5% <0.0%> (+0.8%) ⬆️

Unify duplicated code.

Fix deduplication algorithm.

Fix droppedAttributes to always be returned, even if the span has no
attributes.
@MrAlias
Copy link
Contributor Author

MrAlias commented Feb 3, 2022

Here is a benchmark showing the comparison between main, #2555, and this PR

main#2555this PR
time/op
AttributesMapToKeyValue-83.21µs ±12%
SpanProcessor-814.9µs ±10%12.6µs ± 9%11.3µs ±11%
SpanProcessorVerboseLogging-815.5µs ± 3%12.8µs ±16%12.1µs ± 6%
StartEndSpan/AlwaysSample-8978ns ±14%708ns ±24%636ns ± 5%
StartEndSpan/NeverSample-8367ns ± 2%312ns ± 9%313ns ± 2%
SpanSetAttributesOverCapacity-823.1µs ± 5%2.3µs ±13%2.2µs ± 3%
SpanWithAttributes_4/AlwaysSample-81.85µs ± 5%1.28µs ±11%1.18µs ± 3%
SpanWithAttributes_4/NeverSample-8513ns ± 7%481ns ± 5%468ns ± 1%
SpanWithAttributes_8/AlwaysSample-82.68µs ± 5%1.60µs ± 5%1.62µs ± 4%
SpanWithAttributes_8/NeverSample-8679ns ± 5%644ns ± 7%620ns ± 3%
SpanWithAttributes_all/AlwaysSample-82.09µs ± 7%1.36µs ± 3%1.48µs ± 3%
SpanWithAttributes_all/NeverSample-8566ns ± 5%508ns ± 7%507ns ± 6%
SpanWithAttributes_all_2x/AlwaysSample-83.72µs ±12%2.57µs ± 9%2.11µs ± 4%
SpanWithAttributes_all_2x/NeverSample-8753ns ± 6%690ns ± 8%692ns ± 6%
SpanWithEvents_4/AlwaysSample-82.04µs ± 7%1.55µs ± 9%1.50µs ± 2%
SpanWithEvents_4/NeverSample-8386ns ± 3%324ns ± 7%322ns ± 3%
SpanWithEvents_8/AlwaysSample-83.02µs ± 3%2.32µs ± 8%2.27µs ± 1%
SpanWithEvents_8/NeverSample-8395ns ± 8%339ns ± 7%333ns ± 3%
SpanWithEvents_WithStackTrace/AlwaysSample-81.26µs ± 8%1.01µs ±12%0.91µs ± 2%
SpanWithEvents_WithStackTrace/NeverSample-8423ns ±10%378ns ±16%363ns ± 8%
SpanWithEvents_WithTimestamp/AlwaysSample-81.24µs ± 5%0.98µs ± 9%0.92µs ± 2%
SpanWithEvents_WithTimestamp/NeverSample-8470ns ± 2%418ns ± 8%408ns ± 1%
TraceID_DotString-882.2ns ± 5%84.2ns ±10%81.9ns ± 2%
SpanID_DotString-862.3ns ± 2%62.7ns ± 6%61.5ns ± 2%
 
alloc/op
AttributesMapToKeyValue-8
SpanProcessor-812.3kB ± 0%8.6kB ± 0%8.4kB ± 0%
SpanProcessorVerboseLogging-813.4kB ± 0%9.7kB ± 0%9.6kB ± 0%
StartEndSpan/AlwaysSample-8816B ± 0%448B ± 0%432B ± 0%
StartEndSpan/NeverSample-8224B ± 0%128B ± 0%128B ± 0%
SpanSetAttributesOverCapacity-815.4kB ± 0%1.0kB ± 0%0.5kB ± 0%
SpanWithAttributes_4/AlwaysSample-81.73kB ± 0%1.28kB ± 0%1.14kB ± 0%
SpanWithAttributes_4/NeverSample-8480B ± 0%384B ± 0%384B ± 0%
SpanWithAttributes_8/AlwaysSample-82.43kB ± 0%1.54kB ± 0%1.90kB ± 0%
SpanWithAttributes_8/NeverSample-8736B ± 0%640B ± 0%640B ± 0%
SpanWithAttributes_all/AlwaysSample-81.90kB ± 0%1.34kB ± 0%1.71kB ± 0%
SpanWithAttributes_all/NeverSample-8544B ± 0%448B ± 0%448B ± 0%
SpanWithAttributes_all_2x/AlwaysSample-83.20kB ± 0%2.83kB ± 0%3.06kB ± 0%
SpanWithAttributes_all_2x/NeverSample-8864B ± 0%768B ± 0%768B ± 0%
SpanWithEvents_4/AlwaysSample-81.50kB ± 0%0.88kB ± 0%0.86kB ± 0%
SpanWithEvents_4/NeverSample-8224B ± 0%128B ± 0%128B ± 0%
SpanWithEvents_8/AlwaysSample-82.21kB ± 0%1.33kB ± 0%1.31kB ± 0%
SpanWithEvents_8/NeverSample-8224B ± 0%128B ± 0%128B ± 0%
SpanWithEvents_WithStackTrace/AlwaysSample-8992B ± 0%560B ± 0%544B ± 0%
SpanWithEvents_WithStackTrace/NeverSample-8240B ± 0%144B ± 0%144B ± 0%
SpanWithEvents_WithTimestamp/AlwaysSample-81.02kB ± 0%0.58kB ± 0%0.57kB ± 0%
SpanWithEvents_WithTimestamp/NeverSample-8264B ± 0%168B ± 0%168B ± 0%
TraceID_DotString-8
SpanID_DotString-8
 
allocs/op
AttributesMapToKeyValue-8
SpanProcessor-885.0 ± 0%45.0 ± 0%35.0 ± 0%
SpanProcessorVerboseLogging-891.0 ± 0%51.0 ± 0%41.0 ± 0%
StartEndSpan/AlwaysSample-87.00 ± 0%3.00 ± 0%2.00 ± 0%
StartEndSpan/NeverSample-83.00 ± 0%2.00 ± 0%2.00 ± 0%
SpanSetAttributesOverCapacity-8264 ± 0%4 ± 0%3 ± 0%
SpanWithAttributes_4/AlwaysSample-817.0 ± 0%5.0 ± 0%6.0 ± 0%
SpanWithAttributes_4/NeverSample-84.00 ± 0%3.00 ± 0%3.00 ± 0%
SpanWithAttributes_8/AlwaysSample-825.0 ± 0%5.0 ± 0%7.0 ± 0%
SpanWithAttributes_8/NeverSample-84.00 ± 0%3.00 ± 0%3.00 ± 0%
SpanWithAttributes_all/AlwaysSample-819.0 ± 0%5.0 ± 0%7.0 ± 0%
SpanWithAttributes_all/NeverSample-84.00 ± 0%3.00 ± 0%3.00 ± 0%
SpanWithAttributes_all_2x/AlwaysSample-830.0 ± 0%6.0 ± 0%8.0 ± 0%
SpanWithAttributes_all_2x/NeverSample-84.00 ± 0%3.00 ± 0%3.00 ± 0%
SpanWithEvents_4/AlwaysSample-818.0 ± 0%10.0 ± 0%9.0 ± 0%
SpanWithEvents_4/NeverSample-83.00 ± 0%2.00 ± 0%2.00 ± 0%
SpanWithEvents_8/AlwaysSample-827.0 ± 0%15.0 ± 0%14.0 ± 0%
SpanWithEvents_8/NeverSample-83.00 ± 0%2.00 ± 0%2.00 ± 0%
SpanWithEvents_WithStackTrace/AlwaysSample-811.0 ± 0%6.0 ± 0%5.0 ± 0%
SpanWithEvents_WithStackTrace/NeverSample-84.00 ± 0%3.00 ± 0%3.00 ± 0%
SpanWithEvents_WithTimestamp/AlwaysSample-812.0 ± 0%7.0 ± 0%6.0 ± 0%
SpanWithEvents_WithTimestamp/NeverSample-85.00 ± 0%4.00 ± 0%4.00 ± 0%
TraceID_DotString-8
SpanID_DotString-8
 

@MrAlias MrAlias added this to the Release v1.4.0/v0.27.0 milestone Feb 4, 2022
sdk/trace/tracer.go Outdated Show resolved Hide resolved
MrAlias and others added 2 commits February 4, 2022 15:45
Copy link
Contributor

@jmacd jmacd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@MrAlias MrAlias merged commit 98bb105 into open-telemetry:main Feb 7, 2022
@MrAlias MrAlias deleted the recordingSpan-attrs-slice branch February 7, 2022 20:58
@MrAlias MrAlias mentioned this pull request Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Span attribute drop logic does not comply with the specification. Improve Span benchmarks
3 participants