Skip to content

Commit 7ee0b28

Browse files
authored
[pdata] Introduce runtime safeguards to catch incorrect pdata mutations (#8494)
This change introduces an option to enable runtime assertions to catch unintentional pdata mutations in components that are claimed as non-mutating pdata. Without these assertions, runtime errors may still occur, but thrown by unrelated components, making it very difficult to troubleshoot. For now, this doesn't change the default behavior. It just introduces a new method on `[Metrics/Traces|Logs].Shared()` that returns pdata marked as shared. The method will be applied to fan-out consumers in the next PR. Later, if we want to remove the need of `MutatesData` capability, we can introduce another method `[Metrics/Traces|Logs].Exclusive()` which returns a copy of the pdata if it's shared. This change unblocks the 1.0 release by implementing the original solution proposed by @bogdandrutu in #6794. Going forward, we may introduce a copy-on-write solution that doesn't require the runtime assertions. That will likely be part of the 2.0 release.
1 parent b89b819 commit 7ee0b28

File tree

138 files changed

+2358
-541
lines changed

Some content is hidden

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

138 files changed

+2358
-541
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: enhancement
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
5+
component: pdata
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Introduce runtime assertions to catch incorrect pdata mutations
9+
10+
# One or more tracking issues or pull requests related to the change
11+
issues: [6794]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext: |
17+
This change introduces an option to enable runtime assertions to catch unintentional pdata mutations in components
18+
that are claimed as non-mutating pdata. Without these assertions, runtime errors may still occur, but thrown by
19+
unrelated components, making it very difficult to troubleshoot.

pdata/internal/cmd/pdatagen/internal/base_fields.go

Lines changed: 54 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,15 @@ import (
1212
const accessorSliceTemplate = `// {{ .fieldName }} returns the {{ .fieldName }} associated with this {{ .structName }}.
1313
func (ms {{ .structName }}) {{ .fieldName }}() {{ .packageName }}{{ .returnType }} {
1414
{{- if .isCommon }}
15-
return {{ .packageName }}{{ .returnType }}(internal.New{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}))
15+
return {{ .packageName }}{{ .returnType }}(internal.New{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}
16+
{{- if .isBaseStructCommon -}}
17+
, internal.Get{{ .structName }}State(internal.{{ .structName }}(ms))
18+
{{- else -}}
19+
, ms.state
20+
{{- end -}}
21+
))
1622
{{- else }}
17-
return new{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }})
23+
return new{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}, ms.state)
1824
{{- end }}
1925
}`
2026

@@ -36,14 +42,14 @@ const setTestValueTemplate = `{{ if .isCommon -}}
3642
{{- else -}}
3743
fillTest{{ .returnType }}(new
3844
{{- end -}}
39-
{{ .returnType }}(&tv.orig.{{ .fieldName }}))`
45+
{{ .returnType }}(&tv.orig.{{ .fieldName }}, tv.state))`
4046

4147
const accessorsMessageValueTemplate = `// {{ .fieldName }} returns the {{ .lowerFieldName }} associated with this {{ .structName }}.
4248
func (ms {{ .structName }}) {{ .fieldName }}() {{ .packageName }}{{ .returnType }} {
4349
{{- if .isCommon }}
44-
return {{ .packageName }}{{ .returnType }}(internal.New{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}))
50+
return {{ .packageName }}{{ .returnType }}(internal.New{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}, ms.state))
4551
{{- else }}
46-
return new{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }})
52+
return new{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}, ms.state)
4753
{{- end }}
4854
}`
4955

@@ -65,12 +71,13 @@ func (ms {{ .structName }}) {{ .fieldName }}() {{ .packageName }}{{ .returnType
6571
6672
// Set{{ .fieldName }} replaces the {{ .lowerFieldName }} associated with this {{ .structName }}.
6773
func (ms {{ .structName }}) Set{{ .fieldName }}(v {{ .returnType }}) {
74+
ms.{{ .stateAccessor }}.AssertMutable()
6875
ms.{{ .origAccessor }}.{{ .fieldName }} = v
6976
}`
7077

7178
const accessorsPrimitiveSliceTemplate = `// {{ .fieldName }} returns the {{ .lowerFieldName }} associated with this {{ .structName }}.
7279
func (ms {{ .structName }}) {{ .fieldName }}() {{ .packageName }}{{ .returnType }} {
73-
return {{ .packageName }}{{ .returnType }}(internal.New{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}))
80+
return {{ .packageName }}{{ .returnType }}(internal.New{{ .returnType }}(&ms.{{ .origAccessor }}.{{ .fieldName }}, ms.state))
7481
}`
7582

7683
const oneOfTypeAccessorTemplate = `// {{ .typeFuncName }} returns the type of the {{ .lowerOriginFieldName }} for this {{ .structName }}.
@@ -108,7 +115,7 @@ func (ms {{ .structName }}) {{ .fieldName }}() {{ .returnType }} {
108115
if !ok {
109116
return {{ .returnType }}{}
110117
}
111-
return new{{ .returnType }}(v.{{ .fieldName }})
118+
return new{{ .returnType }}(v.{{ .fieldName }}, ms.state)
112119
}
113120
114121
// SetEmpty{{ .fieldName }} sets an empty {{ .lowerFieldName }} to this {{ .structName }}.
@@ -117,16 +124,19 @@ func (ms {{ .structName }}) {{ .fieldName }}() {{ .returnType }} {
117124
//
118125
// Calling this function on zero-initialized {{ .structName }} will cause a panic.
119126
func (ms {{ .structName }}) SetEmpty{{ .fieldName }}() {{ .returnType }} {
127+
ms.state.AssertMutable()
120128
val := &{{ .originFieldPackageName }}.{{ .fieldName }}{}
121129
ms.orig.{{ .originOneOfFieldName }} = &{{ .originStructType }}{{ "{" }}{{ .fieldName }}: val}
122-
return new{{ .returnType }}(val)
130+
return new{{ .returnType }}(val, ms.state)
123131
}`
124132

125133
const accessorsOneOfMessageTestTemplate = `func Test{{ .structName }}_{{ .fieldName }}(t *testing.T) {
126134
ms := New{{ .structName }}()
127135
fillTest{{ .returnType }}(ms.SetEmpty{{ .fieldName }}())
128136
assert.Equal(t, {{ .typeName }}, ms.{{ .originOneOfTypeFuncName }}())
129137
assert.Equal(t, generateTest{{ .returnType }}(), ms.{{ .fieldName }}())
138+
sharedState := internal.StateReadOnly
139+
assert.Panics(t, func() { new{{ .structName }}(&{{ .originStructName }}{}, &sharedState).SetEmpty{{ .fieldName }}() })
130140
}
131141
132142
func Test{{ .structName }}_CopyTo_{{ .fieldName }}(t *testing.T) {
@@ -135,6 +145,8 @@ func Test{{ .structName }}_CopyTo_{{ .fieldName }}(t *testing.T) {
135145
dest := New{{ .structName }}()
136146
ms.CopyTo(dest)
137147
assert.Equal(t, ms, dest)
148+
sharedState := internal.StateReadOnly
149+
assert.Panics(t, func() { ms.CopyTo(new{{ .structName }}(&{{ .originStructName }}{}, &sharedState)) })
138150
}`
139151

140152
const copyToValueOneOfMessageTemplate = ` case {{ .typeName }}:
@@ -147,6 +159,7 @@ func (ms {{ .structName }}) {{ .accessorFieldName }}() {{ .returnType }} {
147159
148160
// Set{{ .accessorFieldName }} replaces the {{ .lowerFieldName }} associated with this {{ .structName }}.
149161
func (ms {{ .structName }}) Set{{ .accessorFieldName }}(v {{ .returnType }}) {
162+
ms.state.AssertMutable()
150163
ms.orig.{{ .originOneOfFieldName }} = &{{ .originStructType }}{
151164
{{ .originFieldName }}: v,
152165
}
@@ -158,13 +171,17 @@ const accessorsOneOfPrimitiveTestTemplate = `func Test{{ .structName }}_{{ .acce
158171
ms.Set{{ .accessorFieldName }}({{ .testValue }})
159172
assert.Equal(t, {{ .testValue }}, ms.{{ .accessorFieldName }}())
160173
assert.Equal(t, {{ .typeName }}, ms.{{ .originOneOfTypeFuncName }}())
174+
sharedState := internal.StateReadOnly
175+
assert.Panics(t, func() { new{{ .structName }}(&{{ .originStructName }}{}, &sharedState).Set{{ .accessorFieldName }}({{ .testValue }}) })
161176
}`
162177

163178
const accessorsPrimitiveTestTemplate = `func Test{{ .structName }}_{{ .fieldName }}(t *testing.T) {
164179
ms := New{{ .structName }}()
165180
assert.Equal(t, {{ .defaultVal }}, ms.{{ .fieldName }}())
166181
ms.Set{{ .fieldName }}({{ .testValue }})
167182
assert.Equal(t, {{ .testValue }}, ms.{{ .fieldName }}())
183+
sharedState := internal.StateReadOnly
184+
assert.Panics(t, func() { new{{ .structName }}(&{{ .originStructName }}{}, &sharedState).Set{{ .fieldName }}({{ .testValue }}) })
168185
}`
169186

170187
const accessorsPrimitiveTypedTemplate = `// {{ .fieldName }} returns the {{ .lowerFieldName }} associated with this {{ .structName }}.
@@ -174,6 +191,7 @@ func (ms {{ .structName }}) {{ .fieldName }}() {{ .packageName }}{{ .returnType
174191
175192
// Set{{ .fieldName }} replaces the {{ .lowerFieldName }} associated with this {{ .structName }}.
176193
func (ms {{ .structName }}) Set{{ .fieldName }}(v {{ .packageName }}{{ .returnType }}) {
194+
ms.state.AssertMutable()
177195
ms.orig.{{ .originFieldName }} = {{ .rawType }}(v)
178196
}`
179197

@@ -205,11 +223,13 @@ func (ms {{ .structName }}) Has{{ .fieldName }}() bool {
205223
206224
// Set{{ .fieldName }} replaces the {{ .lowerFieldName }} associated with this {{ .structName }}.
207225
func (ms {{ .structName }}) Set{{ .fieldName }}(v {{ .returnType }}) {
226+
ms.state.AssertMutable()
208227
ms.orig.{{ .fieldName }}_ = &{{ .originStructType }}{{ "{" }}{{ .fieldName }}: v}
209228
}
210229
211230
// Remove{{ .fieldName }} removes the {{ .lowerFieldName }} associated with this {{ .structName }}.
212231
func (ms {{ .structName }}) Remove{{ .fieldName }}() {
232+
ms.state.AssertMutable()
213233
ms.orig.{{ .fieldName }}_ = nil
214234
}`
215235

@@ -281,6 +301,7 @@ func (sf *sliceField) templateFields(ms *messageValueStruct) map[string]any {
281301
}(),
282302
"returnType": sf.returnSlice.getName(),
283303
"origAccessor": origAccessor(ms),
304+
"stateAccessor": stateAccessor(ms),
284305
"isCommon": usedByOtherDataTypes(sf.returnSlice.getPackageName()),
285306
"isBaseStructCommon": usedByOtherDataTypes(ms.packageName),
286307
}
@@ -337,7 +358,8 @@ func (mf *messageValueField) templateFields(ms *messageValueStruct) map[string]a
337358
}
338359
return ""
339360
}(),
340-
"origAccessor": origAccessor(ms),
361+
"origAccessor": origAccessor(ms),
362+
"stateAccessor": stateAccessor(ms),
341363
}
342364
}
343365

@@ -378,14 +400,16 @@ func (pf *primitiveField) GenerateCopyToValue(_ *messageValueStruct) string {
378400

379401
func (pf *primitiveField) templateFields(ms *messageValueStruct) map[string]any {
380402
return map[string]any{
381-
"structName": ms.getName(),
382-
"packageName": "",
383-
"defaultVal": pf.defaultVal,
384-
"fieldName": pf.fieldName,
385-
"lowerFieldName": strings.ToLower(pf.fieldName),
386-
"testValue": pf.testVal,
387-
"returnType": pf.returnType,
388-
"origAccessor": origAccessor(ms),
403+
"structName": ms.getName(),
404+
"packageName": "",
405+
"defaultVal": pf.defaultVal,
406+
"fieldName": pf.fieldName,
407+
"lowerFieldName": strings.ToLower(pf.fieldName),
408+
"testValue": pf.testVal,
409+
"returnType": pf.returnType,
410+
"origAccessor": origAccessor(ms),
411+
"stateAccessor": stateAccessor(ms),
412+
"originStructName": ms.originFullName,
389413
}
390414
}
391415

@@ -513,6 +537,7 @@ func (psf *primitiveSliceField) templateFields(ms *messageValueStruct) map[strin
513537
"lowerFieldName": strings.ToLower(psf.fieldName),
514538
"testValue": psf.testVal,
515539
"origAccessor": origAccessor(ms),
540+
"stateAccessor": stateAccessor(ms),
516541
}
517542
}
518543

@@ -576,6 +601,7 @@ func (of *oneOfField) templateFields(ms *messageValueStruct) map[string]any {
576601
"originFieldName": of.originFieldName,
577602
"lowerOriginFieldName": strings.ToLower(of.originFieldName),
578603
"origAccessor": origAccessor(ms),
604+
"stateAccessor": stateAccessor(ms),
579605
"values": of.values,
580606
"originTypePrefix": ms.originFullName + "_",
581607
}
@@ -653,6 +679,7 @@ func (opv *oneOfPrimitiveValue) templateFields(ms *messageValueStruct, of *oneOf
653679
"returnType": opv.returnType,
654680
"originFieldName": opv.originFieldName,
655681
"originOneOfFieldName": of.originFieldName,
682+
"originStructName": ms.originFullName,
656683
"originStructType": ms.originFullName + "_" + opv.originFieldName,
657684
}
658685
}
@@ -687,7 +714,7 @@ func (omv *oneOfMessageValue) GenerateTests(ms *messageValueStruct, of *oneOfFie
687714
func (omv *oneOfMessageValue) GenerateSetWithTestValue(ms *messageValueStruct, of *oneOfField) string {
688715
return "\ttv.orig." + of.originFieldName + " = &" + ms.originFullName + "_" + omv.fieldName + "{" + omv.
689716
fieldName + ": &" + omv.originFieldPackageName + "." + omv.fieldName + "{}}\n" +
690-
"\tfillTest" + omv.returnMessage.structName + "(new" + omv.fieldName + "(tv.orig.Get" + omv.fieldName + "()))"
717+
"\tfillTest" + omv.returnMessage.structName + "(new" + omv.fieldName + "(tv.orig.Get" + omv.fieldName + "(), tv.state))"
691718
}
692719

693720
func (omv *oneOfMessageValue) GenerateCopyToValue(ms *messageValueStruct, of *oneOfField, sb *bytes.Buffer) {
@@ -713,6 +740,7 @@ func (omv *oneOfMessageValue) templateFields(ms *messageValueStruct, of *oneOfFi
713740
"originOneOfTypeFuncName": of.typeFuncName(),
714741
"lowerFieldName": strings.ToLower(omv.fieldName),
715742
"originFieldPackageName": omv.originFieldPackageName,
743+
"originStructName": ms.originFullName,
716744
"originStructType": ms.originFullName + "_" + omv.fieldName,
717745
}
718746
}
@@ -764,6 +792,7 @@ func (opv *optionalPrimitiveValue) templateFields(ms *messageValueStruct) map[st
764792
"lowerFieldName": strings.ToLower(opv.fieldName),
765793
"testValue": opv.testVal,
766794
"returnType": opv.returnType,
795+
"originStructName": ms.originFullName,
767796
"originStructType": ms.originFullName + "_" + opv.fieldName,
768797
}
769798
}
@@ -776,3 +805,10 @@ func origAccessor(bs *messageValueStruct) string {
776805
}
777806
return "orig"
778807
}
808+
809+
func stateAccessor(bs *messageValueStruct) string {
810+
if usedByOtherDataTypes(bs.packageName) {
811+
return "getState()"
812+
}
813+
return "state"
814+
}

0 commit comments

Comments
 (0)