Skip to content

Commit df2fb46

Browse files
committed
[pdata] Introduce runtime safeguards to catch incorrect pdata mutations
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 difficult to troubleshoot the problem. For now, this doesn't change the default behavior. It just introduces a new method on [Metrics/Traces|Logs].Shared. The method will be applied to fan out consumers in the next PR. This change unblocks the 1.0 release of pdata. Going-forward we may introduce copy-on-write solution which won't require the runtime assertions. That will be likely part of 2.0 release.
1 parent 512b9c3 commit df2fb46

File tree

124 files changed

+1437
-554
lines changed

Some content is hidden

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

124 files changed

+1437
-554
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: 17 additions & 9 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.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 }}.
@@ -119,7 +126,7 @@ func (ms {{ .structName }}) {{ .fieldName }}() {{ .returnType }} {
119126
func (ms {{ .structName }}) SetEmpty{{ .fieldName }}() {{ .returnType }} {
120127
val := &{{ .originFieldPackageName }}.{{ .fieldName }}{}
121128
ms.orig.{{ .originOneOfFieldName }} = &{{ .originStructType }}{{ "{" }}{{ .fieldName }}: val}
122-
return new{{ .returnType }}(val)
129+
return new{{ .returnType }}(val, ms.state)
123130
}`
124131

125132
const accessorsOneOfMessageTestTemplate = `func Test{{ .structName }}_{{ .fieldName }}(t *testing.T) {
@@ -147,6 +154,7 @@ func (ms {{ .structName }}) {{ .accessorFieldName }}() {{ .returnType }} {
147154
148155
// Set{{ .accessorFieldName }} replaces the {{ .lowerFieldName }} associated with this {{ .structName }}.
149156
func (ms {{ .structName }}) Set{{ .accessorFieldName }}(v {{ .returnType }}) {
157+
ms.assertMutable()
150158
ms.orig.{{ .originOneOfFieldName }} = &{{ .originStructType }}{
151159
{{ .originFieldName }}: v,
152160
}
@@ -688,7 +696,7 @@ func (omv *oneOfMessageValue) GenerateTests(ms baseStruct, of *oneOfField) strin
688696
func (omv *oneOfMessageValue) GenerateSetWithTestValue(of *oneOfField) string {
689697
return "\ttv.orig." + of.originFieldName + " = &" + of.originTypePrefix + omv.fieldName + "{" + omv.
690698
fieldName + ": &" + omv.originFieldPackageName + "." + omv.fieldName + "{}}\n" +
691-
"\tfillTest" + omv.returnMessage.structName + "(new" + omv.fieldName + "(tv.orig.Get" + omv.fieldName + "()))"
699+
"\tfillTest" + omv.returnMessage.structName + "(new" + omv.fieldName + "(tv.orig.Get" + omv.fieldName + "(), tv.state))"
692700
}
693701

694702
func (omv *oneOfMessageValue) GenerateCopyToValue(ms baseStruct, of *oneOfField, sb *bytes.Buffer) {

pdata/internal/cmd/pdatagen/internal/base_slices.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,24 @@ const sliceTemplate = `// {{ .structName }} logically represents a slice of {{ .
1717
// Important: zero-initialized instance is not valid for use.
1818
type {{ .structName }} struct {
1919
orig *[]{{ .originElementType }}
20+
state internal.State
2021
}
2122
22-
func new{{ .structName }}(orig *[]{{ .originElementType }}) {{ .structName }} {
23-
return {{ .structName }}{orig}
23+
func new{{ .structName }}(orig *[]{{ .originElementType }}, state internal.State) {{ .structName }} {
24+
return {{ .structName }}{orig: orig, state: state}
2425
}
2526
2627
// New{{ .structName }} creates a {{ .structName }} with 0 elements.
2728
// Can use "EnsureCapacity" to initialize with a given capacity.
2829
func New{{ .structName }}() {{ .structName }} {
2930
orig := []{{ .originElementType }}(nil)
30-
return new{{ .structName }}(&orig)
31+
return new{{ .structName }}(&orig, internal.StateExclusive)
32+
}
33+
34+
func (es {{ .structName }}) assertMutable() {
35+
if es.state == internal.StateShared {
36+
panic("invalid access to shared data")
37+
}
3138
}
3239
3340
// Len returns the number of elements in the slice.
@@ -73,13 +80,16 @@ func (es {{ .structName }}) EnsureCapacity(newCap int) {
7380
// AppendEmpty will append to the end of the slice an empty {{ .elementName }}.
7481
// It returns the newly added {{ .elementName }}.
7582
func (es {{ .structName }}) AppendEmpty() {{ .elementName }} {
83+
es.assertMutable()
7684
*es.orig = append(*es.orig, {{ .emptyOriginElement }})
7785
return es.At(es.Len() - 1)
7886
}
7987
8088
// MoveAndAppendTo moves all elements from the current slice and appends them to the dest.
8189
// The current slice will be cleared.
8290
func (es {{ .structName }}) MoveAndAppendTo(dest {{ .structName }}) {
91+
es.assertMutable()
92+
dest.assertMutable()
8393
if *dest.orig == nil {
8494
// We can simply move the entire vector and avoid any allocations.
8595
*dest.orig = *es.orig
@@ -92,6 +102,7 @@ func (es {{ .structName }}) MoveAndAppendTo(dest {{ .structName }}) {
92102
// RemoveIf calls f sequentially for each element present in the slice.
93103
// If f returns true, the element is removed from the slice.
94104
func (es {{ .structName }}) RemoveIf(f func({{ .elementName }}) bool) {
105+
es.assertMutable()
95106
newLen := 0
96107
for i := 0; i < len(*es.orig); i++ {
97108
if f(es.At(i)) {
@@ -112,22 +123,23 @@ func (es {{ .structName }}) RemoveIf(f func({{ .elementName }}) bool) {
112123
113124
// CopyTo copies all elements from the current slice overriding the destination.
114125
func (es {{ .structName }}) CopyTo(dest {{ .structName }}) {
126+
dest.assertMutable()
115127
srcLen := es.Len()
116128
destCap := cap(*dest.orig)
117129
if srcLen <= destCap {
118130
(*dest.orig) = (*dest.orig)[:srcLen:destCap]
119131
120132
{{- if eq .type "sliceOfPtrs" }}
121133
for i := range *es.orig {
122-
new{{ .elementName }}((*es.orig)[i]).CopyTo(new{{ .elementName }}((*dest.orig)[i]))
134+
new{{ .elementName }}((*es.orig)[i], internal.StateExclusive).CopyTo(new{{ .elementName }}((*dest.orig)[i], internal.StateExclusive))
123135
}
124136
return
125137
}
126138
origs := make([]{{ .originName }}, srcLen)
127139
wrappers := make([]*{{ .originName }}, srcLen)
128140
for i := range *es.orig {
129141
wrappers[i] = &origs[i]
130-
new{{ .elementName }}((*es.orig)[i]).CopyTo(new{{ .elementName }}(wrappers[i]))
142+
new{{ .elementName }}((*es.orig)[i], internal.StateExclusive).CopyTo(new{{ .elementName }}(wrappers[i], internal.StateExclusive))
131143
}
132144
*dest.orig = wrappers
133145
@@ -136,7 +148,7 @@ func (es {{ .structName }}) CopyTo(dest {{ .structName }}) {
136148
(*dest.orig) = make([]{{ .originElementType }}, srcLen)
137149
}
138150
for i := range *es.orig {
139-
{{ .newElement }}.CopyTo(new{{ .elementName }}(&(*dest.orig)[i]))
151+
{{ .newElement }}.CopyTo(new{{ .elementName }}(&(*dest.orig)[i], internal.StateExclusive))
140152
}
141153
{{- end }}
142154
}
@@ -146,14 +158,15 @@ func (es {{ .structName }}) CopyTo(dest {{ .structName }}) {
146158
// provided less function so that two instances of {{ .structName }}
147159
// can be compared.
148160
func (es {{ .structName }}) Sort(less func(a, b {{ .elementName }}) bool) {
161+
es.assertMutable()
149162
sort.SliceStable(*es.orig, func(i, j int) bool { return less(es.At(i), es.At(j)) })
150163
}
151164
{{- end }}`
152165

153166
const sliceTestTemplate = `func Test{{ .structName }}(t *testing.T) {
154167
es := New{{ .structName }}()
155168
assert.Equal(t, 0, es.Len())
156-
es = new{{ .structName }}(&[]{{ .originElementType }}{})
169+
es = new{{ .structName }}(&[]{{ .originElementType }}{}, internal.StateExclusive)
157170
assert.Equal(t, 0, es.Len())
158171
159172
emptyVal := New{{ .elementName }}()
@@ -324,7 +337,7 @@ func (ss *sliceOfPtrs) templateFields() map[string]any {
324337
"originName": ss.element.originFullName,
325338
"originElementType": "*" + ss.element.originFullName,
326339
"emptyOriginElement": "&" + ss.element.originFullName + "{}",
327-
"newElement": "new" + ss.element.structName + "((*es.orig)[i])",
340+
"newElement": "new" + ss.element.structName + "((*es.orig)[i], es.state)",
328341
}
329342
}
330343

@@ -376,7 +389,7 @@ func (ss *sliceOfValues) templateFields() map[string]any {
376389
"originName": ss.element.originFullName,
377390
"originElementType": ss.element.originFullName,
378391
"emptyOriginElement": ss.element.originFullName + "{}",
379-
"newElement": "new" + ss.element.structName + "(&(*es.orig)[i])",
392+
"newElement": "new" + ss.element.structName + "(&(*es.orig)[i], es.state)",
380393
}
381394
}
382395

pdata/internal/cmd/pdatagen/internal/base_structs.go

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,15 @@ type {{ .structName }} internal.{{ .structName }}
2121
{{- else }}
2222
type {{ .structName }} struct {
2323
orig *{{ .originName }}
24+
state internal.State
2425
}
2526
{{- end }}
2627
27-
func new{{ .structName }}(orig *{{ .originName }}) {{ .structName }} {
28+
func new{{ .structName }}(orig *{{ .originName }}, state internal.State) {{ .structName }} {
2829
{{- if .isCommon }}
29-
return {{ .structName }}(internal.New{{ .structName }}(orig))
30+
return {{ .structName }}(internal.New{{ .structName }}(orig, state))
3031
{{- else }}
31-
return {{ .structName }}{orig}
32+
return {{ .structName }}{orig: orig, state: state}
3233
{{- end }}
3334
}
3435
@@ -37,12 +38,24 @@ func new{{ .structName }}(orig *{{ .originName }}) {{ .structName }} {
3738
// This must be used only in testing code. Users should use "AppendEmpty" when part of a Slice,
3839
// OR directly access the member if this is embedded in another struct.
3940
func New{{ .structName }}() {{ .structName }} {
40-
return new{{ .structName }}(&{{ .originName }}{})
41+
return new{{ .structName }}(&{{ .originName }}{}, internal.StateExclusive)
42+
}
43+
44+
func (ms {{ .structName }}) assertMutable() {
45+
{{- if .isCommon }}
46+
if internal.Get{{ .structName }}State(internal.{{ .structName }}(ms)) == internal.StateShared {
47+
{{- else }}
48+
if ms.state == internal.StateShared {
49+
{{- end }}
50+
panic("invalid access to shared data")
51+
}
4152
}
4253
4354
// MoveTo moves all properties from the current struct overriding the destination and
4455
// resetting the current instance to its zero value
4556
func (ms {{ .structName }}) MoveTo(dest {{ .structName }}) {
57+
ms.assertMutable()
58+
dest.assertMutable()
4659
*dest.{{ .origAccessor }} = *ms.{{ .origAccessor }}
4760
*ms.{{ .origAccessor }} = {{ .originName }}{}
4861
}
@@ -59,9 +72,10 @@ func (ms {{ .structName }}) getOrig() *{{ .originName }} {
5972
6073
// CopyTo copies all properties from the current struct overriding the destination.
6174
func (ms {{ .structName }}) CopyTo(dest {{ .structName }}) {
62-
{{- range .fields }}
63-
{{ .GenerateCopyToValue $.messageStruct }}
64-
{{- end }}
75+
dest.assertMutable()
76+
{{- range .fields }}
77+
{{ .GenerateCopyToValue $.messageStruct }}
78+
{{- end }}
6579
}`
6680

6781
const messageValueTestTemplate = `
@@ -91,7 +105,7 @@ const messageValueGenerateTestTemplate = `func {{ upperIfInternal "g" }}enerateT
91105
{{- if .isCommon }}
92106
orig := {{ .originName }}{}
93107
{{- end }}
94-
tv := New{{ .structName }}({{ if .isCommon }}&orig{{ end }})
108+
tv := New{{ .structName }}({{ if .isCommon }}&orig, StateExclusive{{ end }})
95109
{{ upperIfInternal "f" }}illTest{{ .structName }}(tv)
96110
return tv
97111
}
@@ -105,14 +119,19 @@ func {{ upperIfInternal "f" }}illTest{{ .structName }}(tv {{ .structName }}) {
105119
const messageValueAliasTemplate = `
106120
type {{ .structName }} struct {
107121
orig *{{ .originName }}
122+
state State
108123
}
109124
110125
func GetOrig{{ .structName }}(ms {{ .structName }}) *{{ .originName }} {
111126
return ms.orig
112127
}
113128
114-
func New{{ .structName }}(orig *{{ .originName }}) {{ .structName }} {
115-
return {{ .structName }}{orig: orig}
129+
func Get{{ .structName }}State(ms {{ .structName }}) State {
130+
return ms.state
131+
}
132+
133+
func New{{ .structName }}(orig *{{ .originName }}, state State) {{ .structName }} {
134+
return {{ .structName }}{orig: orig, state: state}
116135
}`
117136

118137
type baseStruct interface {

pdata/internal/cmd/pdatagen/internal/primitive_slice_structs.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func (ms {{ .structName }}) getOrig() *[]{{ .itemType }} {
2323
// New{{ .structName }} creates a new empty {{ .structName }}.
2424
func New{{ .structName }}() {{ .structName }} {
2525
orig := []{{ .itemType }}(nil)
26-
return {{ .structName }}(internal.New{{ .structName }}(&orig))
26+
return {{ .structName }}(internal.New{{ .structName }}(&orig, internal.StateExclusive))
2727
}
2828
2929
// AsRaw returns a copy of the []{{ .itemType }} slice.
@@ -33,6 +33,7 @@ func (ms {{ .structName }}) AsRaw() []{{ .itemType }} {
3333
3434
// FromRaw copies raw []{{ .itemType }} into the slice {{ .structName }}.
3535
func (ms {{ .structName }}) FromRaw(val []{{ .itemType }}) {
36+
internal.{{ .structName }}(ms).AssertState()
3637
*ms.getOrig() = copy{{ .structName }}(*ms.getOrig(), val)
3738
}
3839
@@ -51,6 +52,7 @@ func (ms {{ .structName }}) At(i int) {{ .itemType }} {
5152
// SetAt sets {{ .itemType }} item at particular index.
5253
// Equivalent of {{ .lowerStructName }}[i] = val
5354
func (ms {{ .structName }}) SetAt(i int, val {{ .itemType }}) {
55+
internal.{{ .structName }}(ms).AssertState()
5456
(*ms.getOrig())[i] = val
5557
}
5658
@@ -61,6 +63,7 @@ func (ms {{ .structName }}) SetAt(i int, val {{ .itemType }}) {
6163
// copy(buf, {{ .lowerStructName }})
6264
// {{ .lowerStructName }} = buf
6365
func (ms {{ .structName }}) EnsureCapacity(newCap int) {
66+
internal.{{ .structName }}(ms).AssertState()
6467
oldCap := cap(*ms.getOrig())
6568
if newCap <= oldCap {
6669
return
@@ -74,18 +77,22 @@ func (ms {{ .structName }}) EnsureCapacity(newCap int) {
7477
// Append appends extra elements to {{ .structName }}.
7578
// Equivalent of {{ .lowerStructName }} = append({{ .lowerStructName }}, elms...)
7679
func (ms {{ .structName }}) Append(elms ...{{ .itemType }}) {
80+
internal.{{ .structName }}(ms).AssertState()
7781
*ms.getOrig() = append(*ms.getOrig(), elms...)
7882
}
7983
8084
// MoveTo moves all elements from the current slice overriding the destination and
8185
// resetting the current instance to its zero value.
8286
func (ms {{ .structName }}) MoveTo(dest {{ .structName }}) {
87+
internal.{{ .structName }}(ms).AssertState()
88+
internal.{{ .structName }}(dest).AssertState()
8389
*dest.getOrig() = *ms.getOrig()
8490
*ms.getOrig() = nil
8591
}
8692
8793
// CopyTo copies all elements from the current slice overriding the destination.
8894
func (ms {{ .structName }}) CopyTo(dest {{ .structName }}) {
95+
internal.{{ .structName }}(dest).AssertState()
8996
*dest.getOrig() = copy{{ .structName }}(*dest.getOrig(), *ms.getOrig())
9097
}
9198
@@ -144,14 +151,21 @@ func Test{{ .structName }}EnsureCapacity(t *testing.T) {
144151
const primitiveSliceInternalTemplate = `
145152
type {{ .structName }} struct {
146153
orig *[]{{ .itemType }}
154+
state State
147155
}
148156
149157
func GetOrig{{ .structName }}(ms {{ .structName }}) *[]{{ .itemType }} {
150158
return ms.orig
151159
}
152160
153-
func New{{ .structName }}(orig *[]{{ .itemType }}) {{ .structName }} {
154-
return {{ .structName }}{orig: orig}
161+
func (ms {{ .structName }}) AssertState() {
162+
if ms.state == StateShared {
163+
panic("invalid access to shared data")
164+
}
165+
}
166+
167+
func New{{ .structName }}(orig *[]{{ .itemType }}, state State) {{ .structName }} {
168+
return {{ .structName }}{orig: orig, state: state}
155169
}`
156170

157171
// primitiveSliceStruct generates a struct for a slice of primitive value elements. The structs are always generated

0 commit comments

Comments
 (0)