Skip to content

Commit ca22b1d

Browse files
authored
Use pointer when passing TransformContext around or calling into (open-telemetry#44541)
This PR changes only the `ottlspan.TransformContext`, if gets approved will file separate PRs for all contexts. * In OTTL Expr and Getter/Setter the TransformContext is no longer pass as value avoiding a copy of 12 words (96B) every time an Expr/Getter/Setter of even simple Get* on the context itself funcs are called. * Avoid allocating a new pcommon.Map every time, very costly when is not needed. Comparing existing benchmarks: **Before:** ``` BenchmarkTwoSpans BenchmarkTwoSpans/no_processing BenchmarkTwoSpans/no_processing-12 841123 1327 ns/op 2592 B/op 46 allocs/op BenchmarkTwoSpans/set_attribute BenchmarkTwoSpans/set_attribute-12 743674 1600 ns/op 2640 B/op 49 allocs/op BenchmarkTwoSpans/keep_keys_attribute BenchmarkTwoSpans/keep_keys_attribute-12 715513 1685 ns/op 2656 B/op 50 allocs/op BenchmarkTwoSpans/no_match BenchmarkTwoSpans/no_match-12 702061 1554 ns/op 2624 B/op 48 allocs/op BenchmarkTwoSpans/inner_field BenchmarkTwoSpans/inner_field-12 721738 1621 ns/op 2624 B/op 48 allocs/op BenchmarkTwoSpans/inner_field_both_spans BenchmarkTwoSpans/inner_field_both_spans-12 716055 1713 ns/op 2656 B/op 50 allocs/op ``` **After:** ``` BenchmarkTwoSpans BenchmarkTwoSpans/no_processing BenchmarkTwoSpans/no_processing-12 1000000 1244 ns/op 2337 B/op 40 allocs/op BenchmarkTwoSpans/set_attribute BenchmarkTwoSpans/set_attribute-12 885855 1369 ns/op 2385 B/op 43 allocs/op BenchmarkTwoSpans/keep_keys_attribute BenchmarkTwoSpans/keep_keys_attribute-12 812374 1448 ns/op 2401 B/op 44 allocs/op BenchmarkTwoSpans/no_match BenchmarkTwoSpans/no_match-12 886906 1331 ns/op 2369 B/op 42 allocs/op BenchmarkTwoSpans/inner_field BenchmarkTwoSpans/inner_field-12 871096 1374 ns/op 2369 B/op 42 allocs/op BenchmarkTwoSpans/inner_field_both_spans BenchmarkTwoSpans/inner_field_both_spans-12 866323 1417 ns/op 2401 B/op 44 allocs/op ``` Comparing benchmarks that do not need to allocate the data again every iteration and does not change the data to measure only the OTTL overhead: **Before:** ``` BenchmarkSetName BenchmarkSetName-12 9204532 122.0 ns/op 32 B/op 2 allocs/op ``` **After:** ``` BenchmarkSetName BenchmarkSetName-12 3745602 305.7 ns/op 288 B/op 8 allocs/op ``` For the current performance benchmarks which are not allocating new TransformContext, here only benefit is because of the object copy part: **Before:** ``` BenchmarkStatementSequenceExecuteSpans BenchmarkStatementSequenceExecuteSpans/small BenchmarkStatementSequenceExecuteSpans/small-12 1000000 1004 ns/op 264 B/op 16 allocs/op BenchmarkStatementSequenceExecuteSpans/medium BenchmarkStatementSequenceExecuteSpans/medium-12 189889 6194 ns/op 1464 B/op 86 allocs/op BenchmarkStatementSequenceExecuteSpans/large BenchmarkStatementSequenceExecuteSpans/large-12 29114 39433 ns/op 6000 B/op 350 allocs/op ``` **After:** ``` BenchmarkStatementSequenceExecuteSpans BenchmarkStatementSequenceExecuteSpans/small BenchmarkStatementSequenceExecuteSpans/small-12 1915495 603.9 ns/op 264 B/op 16 allocs/op BenchmarkStatementSequenceExecuteSpans/medium BenchmarkStatementSequenceExecuteSpans/medium-12 279411 4292 ns/op 1464 B/op 86 allocs/op BenchmarkStatementSequenceExecuteSpans/large BenchmarkStatementSequenceExecuteSpans/large-12 34627 34848 ns/op 6000 B/op 350 allocs/op ``` Signed-off-by: Bogdan Drutu <[email protected]>
1 parent 9cbe6fb commit ca22b1d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+488
-270
lines changed
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: deprecation
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
7+
component: pkg/ottl
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Use pointer when passing TransformContext around or calling into.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [44541]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
Change Expr/Parser/Getter/Setter and all ottl related funcs to accept pointers to avoid unnecessary copy of a large
20+
TransformContext(96B). Avoid allocating a new pcommon.Map every time a new context is created by using a Borrow/Return
21+
pattern and reuse objects between calls. Deprecated funcs are:
22+
- `ottlspan.NewTransformContext` in favor of `ottlspan.BorrowContext`;
23+
- `filtermprocessor.DefaultSpanFunctions` in favor of `filtermprocessor.DefaultSpanFunctionsNew`
24+
- `filtermprocessor.WithSpanFunctions` in favor of `filtermprocessor.WithSpanFunctionsNew`
25+
- `transformprocessor.DefaultSpanFunctions` in favor of `transformprocessor.DefaultSpanFunctionsNew`
26+
- `transformprocessor.WithSpanFunctions` in favor of `transformprocessor.WithSpanFunctionsNew`
27+
28+
# If your change doesn't affect end users or the exported elements of any package,
29+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
30+
# Optional: The change log or logs in which this entry should be included.
31+
# e.g. '[user]' or '[user, api]'
32+
# Include 'user' if the change is relevant to end users.
33+
# Include 'api' if there is a change to a library API.
34+
# Default: '[user]'
35+
change_logs: [api]

connector/countconnector/connector.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type count struct {
3232
component.StartFunc
3333
component.ShutdownFunc
3434

35-
spansMetricDefs map[string]metricDef[ottlspan.TransformContext]
35+
spansMetricDefs map[string]metricDef[*ottlspan.TransformContext]
3636
spanEventsMetricDefs map[string]metricDef[ottlspanevent.TransformContext]
3737
metricsMetricDefs map[string]metricDef[ottlmetric.TransformContext]
3838
dataPointsMetricDefs map[string]metricDef[ottldatapoint.TransformContext]
@@ -51,7 +51,7 @@ func (c *count) ConsumeTraces(ctx context.Context, td ptrace.Traces) error {
5151
for i := 0; i < td.ResourceSpans().Len(); i++ {
5252
resourceSpan := td.ResourceSpans().At(i)
5353
resourceAttrs := resourceSpan.Resource().Attributes()
54-
spansCounter := newCounter[ottlspan.TransformContext](c.spansMetricDefs)
54+
spansCounter := newCounter[*ottlspan.TransformContext](c.spansMetricDefs)
5555
spanEventsCounter := newCounter[ottlspanevent.TransformContext](c.spanEventsMetricDefs)
5656

5757
for j := 0; j < resourceSpan.ScopeSpans().Len(); j++ {
@@ -62,8 +62,9 @@ func (c *count) ConsumeTraces(ctx context.Context, td ptrace.Traces) error {
6262
span := scopeSpan.Spans().At(k)
6363
spansCounter.updateTimestamp(span.StartTimestamp())
6464
spansCounter.updateTimestamp(span.EndTimestamp())
65-
sCtx := ottlspan.NewTransformContext(span, scopeSpan.Scope(), resourceSpan.Resource(), scopeSpan, resourceSpan)
65+
sCtx := ottlspan.NewTransformContextPtr(span, scopeSpan.Scope(), resourceSpan.Resource(), scopeSpan, resourceSpan)
6666
multiError = errors.Join(multiError, spansCounter.update(ctx, span.Attributes(), scopeAttrs, resourceAttrs, sCtx))
67+
sCtx.Close()
6768

6869
for l := 0; l < span.Events().Len(); l++ {
6970
event := span.Events().At(l)

connector/countconnector/counter_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import (
1414
)
1515

1616
func Test_update_attribute_inheritance(t *testing.T) {
17-
spanMetricDefs := make(map[string]metricDef[ottlspan.TransformContext])
18-
spanMetricDefs[defaultMetricNameSpans] = metricDef[ottlspan.TransformContext]{
17+
spanMetricDefs := make(map[string]metricDef[*ottlspan.TransformContext])
18+
spanMetricDefs[defaultMetricNameSpans] = metricDef[*ottlspan.TransformContext]{
1919
desc: defaultMetricDescSpans,
2020
attrs: []AttributeConfig{
2121
{
@@ -129,8 +129,8 @@ func Test_update_attribute_inheritance(t *testing.T) {
129129

130130
for _, tt := range tests {
131131
t.Run(tt.name, func(t *testing.T) {
132-
spansCounter := newCounter[ottlspan.TransformContext](spanMetricDefs)
133-
err := spansCounter.update(t.Context(), tt.spanAttr, tt.scopeAttr, tt.resourceAttr, ottlspan.TransformContext{})
132+
spansCounter := newCounter[*ottlspan.TransformContext](spanMetricDefs)
133+
err := spansCounter.update(t.Context(), tt.spanAttr, tt.scopeAttr, tt.resourceAttr, &ottlspan.TransformContext{})
134134
require.NoError(t, err)
135135
require.NotNil(t, spansCounter)
136136
m := spansCounter.counts[defaultMetricNameSpans]
@@ -215,15 +215,15 @@ func Test_update_attributes_types(t *testing.T) {
215215
}
216216

217217
for _, tt := range tests {
218-
spanMetricDefs := make(map[string]metricDef[ottlspan.TransformContext])
219-
spanMetricDefs[defaultMetricNameSpans] = metricDef[ottlspan.TransformContext]{
218+
spanMetricDefs := make(map[string]metricDef[*ottlspan.TransformContext])
219+
spanMetricDefs[defaultMetricNameSpans] = metricDef[*ottlspan.TransformContext]{
220220
desc: defaultMetricDescSpans,
221221
attrs: tt.config,
222222
}
223223

224224
t.Run(tt.name, func(t *testing.T) {
225225
spansCounter := newCounter(spanMetricDefs)
226-
err := spansCounter.update(t.Context(), pcommon.NewMap(), pcommon.NewMap(), tt.inputAttr, ottlspan.TransformContext{})
226+
err := spansCounter.update(t.Context(), pcommon.NewMap(), pcommon.NewMap(), tt.inputAttr, &ottlspan.TransformContext{})
227227
require.NoError(t, err)
228228
require.NotNil(t, spansCounter)
229229
m := spansCounter.counts[defaultMetricNameSpans]

connector/countconnector/factory.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ func createTracesToMetrics(
5050
) (connector.Traces, error) {
5151
c := cfg.(*Config)
5252

53-
spanMetricDefs := make(map[string]metricDef[ottlspan.TransformContext], len(c.Spans))
53+
spanMetricDefs := make(map[string]metricDef[*ottlspan.TransformContext], len(c.Spans))
5454
for name, info := range c.Spans {
55-
md := metricDef[ottlspan.TransformContext]{
55+
md := metricDef[*ottlspan.TransformContext]{
5656
desc: info.Description,
5757
attrs: info.Attributes,
5858
}

connector/routingconnector/functions.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ func standardFunctions[K any]() map[string]ottl.Factory[K] {
3333
return funcs
3434
}
3535

36-
func spanFunctions() map[string]ottl.Factory[ottlspan.TransformContext] {
37-
funcs := standardFunctions[ottlspan.TransformContext]()
36+
func spanFunctions() map[string]ottl.Factory[*ottlspan.TransformContext] {
37+
funcs := standardFunctions[*ottlspan.TransformContext]()
3838

39-
isRootSpan := ottlfuncs.NewIsRootSpanFactory()
39+
isRootSpan := ottlfuncs.NewIsRootSpanFactoryNew()
4040
funcs[isRootSpan.Name()] = isRootSpan
4141

4242
return funcs

connector/routingconnector/functions_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestSpanFunctions(t *testing.T) {
2222
funcs := spanFunctions()
2323
assertStandardFunctions(t, funcs)
2424

25-
isRouteFuncName := ottlfuncs.NewIsRootSpanFactory().Name()
25+
isRouteFuncName := ottlfuncs.NewIsRootSpanFactoryNew().Name()
2626
val, ok := funcs[isRouteFuncName]
2727

2828
require.True(t, ok)

connector/routingconnector/router.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type consumerProvider[C any] func(...pipeline.ID) (C, error)
3232
// consumer.Logs.
3333
type router[C any] struct {
3434
resourceParser ottl.Parser[ottlresource.TransformContext]
35-
spanParser ottl.Parser[ottlspan.TransformContext]
35+
spanParser ottl.Parser[*ottlspan.TransformContext]
3636
metricParser ottl.Parser[ottlmetric.TransformContext]
3737
dataPointParser ottl.Parser[ottldatapoint.TransformContext]
3838
logParser ottl.Parser[ottllog.TransformContext]
@@ -74,7 +74,7 @@ type routingItem[C any] struct {
7474
consumer C
7575
requestCondition *requestCondition
7676
resourceStatement *ottl.Statement[ottlresource.TransformContext]
77-
spanStatement *ottl.Statement[ottlspan.TransformContext]
77+
spanStatement *ottl.Statement[*ottlspan.TransformContext]
7878
metricStatement *ottl.Statement[ottlmetric.TransformContext]
7979
dataPointStatement *ottl.Statement[ottldatapoint.TransformContext]
8080
logStatement *ottl.Statement[ottllog.TransformContext]

connector/routingconnector/traces.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ func (c *tracesConnector) ConsumeTraces(ctx context.Context, td ptrace.Traces) e
8787
case "span":
8888
ptraceutil.MoveSpansWithContextIf(td, matched,
8989
func(rs ptrace.ResourceSpans, ss ptrace.ScopeSpans, s ptrace.Span) bool {
90-
mtx := ottlspan.NewTransformContext(s, ss.Scope(), rs.Resource(), ss, rs)
90+
mtx := ottlspan.NewTransformContextPtr(s, ss.Scope(), rs.Resource(), ss, rs)
91+
defer mtx.Close()
9192
_, isMatch, err := route.spanStatement.Execute(ctx, mtx)
9293
// If error during statement evaluation consider it as not a match.
9394
if err != nil {

connector/signaltometricsconnector/connector.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type signalToMetrics struct {
2929
collectorInstanceInfo model.CollectorInstanceInfo
3030
logger *zap.Logger
3131

32-
spanMetricDefs []model.MetricDef[ottlspan.TransformContext]
32+
spanMetricDefs []model.MetricDef[*ottlspan.TransformContext]
3333
dpMetricDefs []model.MetricDef[ottldatapoint.TransformContext]
3434
logMetricDefs []model.MetricDef[ottllog.TransformContext]
3535
profileMetricDefs []model.MetricDef[ottlprofile.TransformContext]
@@ -49,7 +49,7 @@ func (sm *signalToMetrics) ConsumeTraces(ctx context.Context, td ptrace.Traces)
4949

5050
processedMetrics := pmetric.NewMetrics()
5151
processedMetrics.ResourceMetrics().EnsureCapacity(td.ResourceSpans().Len())
52-
aggregator := aggregator.NewAggregator[ottlspan.TransformContext](processedMetrics)
52+
aggregator := aggregator.NewAggregator[*ottlspan.TransformContext](processedMetrics)
5353

5454
for i := 0; i < td.ResourceSpans().Len(); i++ {
5555
resourceSpan := td.ResourceSpans().At(i)
@@ -67,20 +67,24 @@ func (sm *signalToMetrics) ConsumeTraces(ctx context.Context, td ptrace.Traces)
6767

6868
// The transform context is created from original attributes so that the
6969
// OTTL expressions are also applied on the original attributes.
70-
tCtx := ottlspan.NewTransformContext(span, scopeSpan.Scope(), resourceSpan.Resource(), scopeSpan, resourceSpan)
70+
tCtx := ottlspan.NewTransformContextPtr(span, scopeSpan.Scope(), resourceSpan.Resource(), scopeSpan, resourceSpan)
7171
if md.Conditions != nil {
7272
match, err := md.Conditions.Eval(ctx, tCtx)
7373
if err != nil {
74+
tCtx.Close()
7475
return fmt.Errorf("failed to evaluate conditions: %w", err)
7576
}
7677
if !match {
78+
tCtx.Close()
7779
sm.logger.Debug("condition not matched, skipping", zap.String("name", md.Key.Name))
7880
continue
7981
}
8082
}
8183

8284
filteredResAttrs := md.FilterResourceAttributes(resourceAttrs, sm.collectorInstanceInfo)
83-
if err := aggregator.Aggregate(ctx, tCtx, md, filteredResAttrs, filteredSpanAttrs, 1); err != nil {
85+
err := aggregator.Aggregate(ctx, tCtx, md, filteredResAttrs, filteredSpanAttrs, 1)
86+
tCtx.Close()
87+
if err != nil {
8488
return err
8589
}
8690
}

connector/signaltometricsconnector/factory.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ func createTracesToMetrics(
5050
return nil, fmt.Errorf("failed to create OTTL statement parser for spans: %w", err)
5151
}
5252

53-
metricDefs := make([]model.MetricDef[ottlspan.TransformContext], 0, len(c.Spans))
53+
metricDefs := make([]model.MetricDef[*ottlspan.TransformContext], 0, len(c.Spans))
5454
for i := range c.Spans {
5555
info := c.Spans[i]
56-
var md model.MetricDef[ottlspan.TransformContext]
56+
var md model.MetricDef[*ottlspan.TransformContext]
5757
if err := md.FromMetricInfo(info, parser, set.TelemetrySettings); err != nil {
5858
return nil, fmt.Errorf("failed to parse provided metric information; %w", err)
5959
}

0 commit comments

Comments
 (0)