Skip to content

Commit

Permalink
Expose non-nullable elements from slices to pointers
Browse files Browse the repository at this point in the history
This is possible because:
* Gogo proto (and protobuf) will not unmarshal any nil element in a slice;
* Our APIs to add elements/remove elements from the slice will guarantee that we never have a nil element in the slice between [0, len-1];

This is an important change because will allow us to change the internal representation (use slice of pointers or non-pointers) without affecting the public API.

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu committed Nov 21, 2020
1 parent 1376e55 commit d2aa6cd
Show file tree
Hide file tree
Showing 65 changed files with 492 additions and 2,070 deletions.
2 changes: 1 addition & 1 deletion cmd/pdatagen/internal/base_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ func (one oneofField) generateSetWithTestValue(sb *strings.Builder) {
}

func (one oneofField) generateCopyToValue(sb *strings.Builder) {
sb.WriteString("\t" + one.copyFuncName + "((*ms.orig), (*dest.orig))")
sb.WriteString("\t" + one.copyFuncName + "(ms.orig, dest.orig)")
}

var _ baseField = (*oneofField)(nil)
50 changes: 27 additions & 23 deletions cmd/pdatagen/internal/base_slices.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package internal

import (
Expand Down Expand Up @@ -44,7 +58,7 @@ func (es ${structName}) Len() int {
// ... // Do something with the element
// }
func (es ${structName}) At(ix int) ${elementName} {
return new${elementName}(&(*es.orig)[ix])
return new${elementName}((*es.orig)[ix])
}
// MoveAndAppendTo moves all elements from the current slice and appends them to the dest.
Expand All @@ -66,15 +80,15 @@ func (es ${structName}) CopyTo(dest ${structName}) {
if srcLen <= destCap {
(*dest.orig) = (*dest.orig)[:srcLen:destCap]
for i := range *es.orig {
new${elementName}(&(*es.orig)[i]).CopyTo(new${elementName}(&(*dest.orig)[i]))
new${elementName}((*es.orig)[i]).CopyTo(new${elementName}((*dest.orig)[i]))
}
return
}
origs := make([]${originName}, srcLen)
wrappers := make([]*${originName}, srcLen)
for i := range *es.orig {
wrappers[i] = &origs[i]
new${elementName}(&(*es.orig)[i]).CopyTo(new${elementName}(&wrappers[i]))
new${elementName}((*es.orig)[i]).CopyTo(new${elementName}(wrappers[i]))
}
*dest.orig = wrappers
}
Expand Down Expand Up @@ -116,7 +130,7 @@ func (es ${structName}) Resize(newLen int) {
// could still be referenced so do not reuse it after passing it to this
// method.
func (es ${structName}) Append(e ${elementName}) {
*es.orig = append(*es.orig, *e.orig)
*es.orig = append(*es.orig, e.orig)
}`

const slicePtrTestTemplate = `func Test${structName}(t *testing.T) {
Expand All @@ -127,7 +141,6 @@ const slicePtrTestTemplate = `func Test${structName}(t *testing.T) {
es.Resize(7)
emptyVal := New${elementName}()
emptyVal.InitEmpty()
testVal := generateTest${elementName}()
assert.EqualValues(t, 7, es.Len())
for i := 0; i < es.Len(); i++ {
Expand Down Expand Up @@ -180,19 +193,18 @@ func Test${structName}_CopyTo(t *testing.T) {
func Test${structName}_Resize(t *testing.T) {
es := generateTest${structName}()
emptyVal := New${elementName}()
emptyVal.InitEmpty()
// Test Resize less elements.
const resizeSmallLen = 4
expectedEs := make(map[*${originName}]bool, resizeSmallLen)
for i := 0; i < resizeSmallLen; i++ {
expectedEs[*(es.At(i).orig)] = true
expectedEs[es.At(i).orig] = true
}
assert.Equal(t, resizeSmallLen, len(expectedEs))
es.Resize(resizeSmallLen)
assert.Equal(t, resizeSmallLen, es.Len())
foundEs := make(map[*${originName}]bool, resizeSmallLen)
for i := 0; i < es.Len(); i++ {
foundEs[*(es.At(i).orig)] = true
foundEs[es.At(i).orig] = true
}
assert.EqualValues(t, expectedEs, foundEs)
Expand All @@ -201,14 +213,14 @@ func Test${structName}_Resize(t *testing.T) {
oldLen := es.Len()
expectedEs = make(map[*${originName}]bool, oldLen)
for i := 0; i < oldLen; i++ {
expectedEs[*(es.At(i).orig)] = true
expectedEs[es.At(i).orig] = true
}
assert.Equal(t, oldLen, len(expectedEs))
es.Resize(resizeLargeLen)
assert.Equal(t, resizeLargeLen, es.Len())
foundEs = make(map[*${originName}]bool, oldLen)
for i := 0; i < oldLen; i++ {
foundEs[*(es.At(i).orig)] = true
foundEs[es.At(i).orig] = true
}
assert.EqualValues(t, expectedEs, foundEs)
for i := oldLen; i < resizeLargeLen; i++ {
Expand All @@ -222,17 +234,14 @@ func Test${structName}_Resize(t *testing.T) {
func Test${structName}_Append(t *testing.T) {
es := generateTest${structName}()
emptyVal := New${elementName}()
emptyVal.InitEmpty()
emptyVal := New${elementName}()
es.Append(emptyVal)
assert.EqualValues(t, *(es.At(7)).orig, *emptyVal.orig)
assert.EqualValues(t, es.At(7).orig, emptyVal.orig)
emptyVal2 := New${elementName}()
emptyVal2.InitEmpty()
es.Append(emptyVal2)
assert.EqualValues(t, *(es.At(8)).orig, *emptyVal2.orig)
assert.EqualValues(t, es.At(8).orig, emptyVal2.orig)
assert.Equal(t, 9, es.Len())
}`
Expand Down Expand Up @@ -367,7 +376,6 @@ const sliceValueTestTemplate = `func Test${structName}(t *testing.T) {
es.Resize(7)
emptyVal := New${elementName}()
emptyVal.InitEmpty()
testVal := generateTest${elementName}()
assert.EqualValues(t, 7, es.Len())
for i := 0; i < es.Len(); i++ {
Expand Down Expand Up @@ -420,7 +428,6 @@ func Test${structName}_CopyTo(t *testing.T) {
func Test${structName}_Resize(t *testing.T) {
es := generateTest${structName}()
emptyVal := New${elementName}()
emptyVal.InitEmpty()
// Test Resize less elements.
const resizeSmallLen = 4
expectedEs := make(map[*${originName}]bool, resizeSmallLen)
Expand Down Expand Up @@ -462,15 +469,12 @@ func Test${structName}_Resize(t *testing.T) {
func Test${structName}_Append(t *testing.T) {
es := generateTest${structName}()
emptyVal := New${elementName}()
emptyVal.InitEmpty()
emptyVal := New${elementName}()
es.Append(emptyVal)
assert.EqualValues(t, *(es.At(7)).orig, *emptyVal.orig)
emptyVal2 := New${elementName}()
emptyVal2.InitEmpty()
es.Append(emptyVal2)
assert.EqualValues(t, *(es.At(8)).orig, *emptyVal2.orig)
Expand All @@ -493,7 +497,7 @@ func fillTest${structName}(tv ${structName}) {
// Will generate code only for the slice struct.
type slicePtrStruct struct {
structName string
element *messagePtrStruct
element *messageValueStruct
}

func (ss *slicePtrStruct) getName() string {
Expand Down
4 changes: 2 additions & 2 deletions cmd/pdatagen/internal/common_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ var stringMap = &slicePtrStruct{
element: stringKeyValue,
}

var stringKeyValue = &messagePtrStruct{}
var stringKeyValue = &messageValueStruct{}

// This will not be generated by this class.
// Defined here just to be available as returned message for the fields.
Expand All @@ -64,7 +64,7 @@ var attributeMap = &slicePtrStruct{
element: attributeKeyValue,
}

var attributeKeyValue = &messagePtrStruct{}
var attributeKeyValue = &messageValueStruct{}

var instrumentationLibraryField = &messagePtrField{
fieldName: "InstrumentationLibrary",
Expand Down
6 changes: 3 additions & 3 deletions cmd/pdatagen/internal/log_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var resourceLogsSlice = &slicePtrStruct{
element: resourceLogs,
}

var resourceLogs = &messagePtrStruct{
var resourceLogs = &messageValueStruct{
structName: "ResourceLogs",
description: "// ResourceLogs is a collection of logs from a Resource.",
originFullName: "otlplogs.ResourceLogs",
Expand All @@ -61,7 +61,7 @@ var instrumentationLibraryLogsSlice = &slicePtrStruct{
element: instrumentationLibraryLogs,
}

var instrumentationLibraryLogs = &messagePtrStruct{
var instrumentationLibraryLogs = &messageValueStruct{
structName: "InstrumentationLibraryLogs",
description: "// InstrumentationLibraryLogs is a collection of logs from a LibraryInstrumentation.",
originFullName: "otlplogs.InstrumentationLibraryLogs",
Expand All @@ -80,7 +80,7 @@ var logSlice = &slicePtrStruct{
element: logRecord,
}

var logRecord = &messagePtrStruct{
var logRecord = &messageValueStruct{
structName: "LogRecord",
description: "// LogRecord are experimental implementation of OpenTelemetry Log Data Model.\n",
originFullName: "otlplogs.LogRecord",
Expand Down
22 changes: 11 additions & 11 deletions cmd/pdatagen/internal/metrics_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ var resourceMetricsSlice = &slicePtrStruct{
element: resourceMetrics,
}

var resourceMetrics = &messagePtrStruct{
var resourceMetrics = &messageValueStruct{
structName: "ResourceMetrics",
description: "// InstrumentationLibraryMetrics is a collection of metrics from a LibraryInstrumentation.",
originFullName: "otlpmetrics.ResourceMetrics",
Expand All @@ -83,7 +83,7 @@ var instrumentationLibraryMetricsSlice = &slicePtrStruct{
element: instrumentationLibraryMetrics,
}

var instrumentationLibraryMetrics = &messagePtrStruct{
var instrumentationLibraryMetrics = &messageValueStruct{
structName: "InstrumentationLibraryMetrics",
description: "// InstrumentationLibraryMetrics is a collection of metrics from a LibraryInstrumentation.",
originFullName: "otlpmetrics.InstrumentationLibraryMetrics",
Expand All @@ -102,7 +102,7 @@ var metricSlice = &slicePtrStruct{
element: metric,
}

var metric = &messagePtrStruct{
var metric = &messageValueStruct{
structName: "Metric",
description: "// Metric represents one metric as a collection of datapoints.\n" +
"// See Metric definition in OTLP: https://github.com/open-telemetry/opentelemetry-proto/blob/master/opentelemetry/proto/metrics/v1/metrics.proto",
Expand Down Expand Up @@ -229,7 +229,7 @@ var intDataPointSlice = &slicePtrStruct{
element: intDataPoint,
}

var intDataPoint = &messagePtrStruct{
var intDataPoint = &messageValueStruct{
structName: "IntDataPoint",
description: "// IntDataPoint is a single data point in a timeseries that describes the time-varying values of a scalar int metric.",
originFullName: "otlpmetrics.IntDataPoint",
Expand All @@ -247,7 +247,7 @@ var doubleDataPointSlice = &slicePtrStruct{
element: doubleDataPoint,
}

var doubleDataPoint = &messagePtrStruct{
var doubleDataPoint = &messageValueStruct{
structName: "DoubleDataPoint",
description: "// DoubleDataPoint is a single data point in a timeseries that describes the time-varying value of a double metric.",
originFullName: "otlpmetrics.DoubleDataPoint",
Expand All @@ -265,7 +265,7 @@ var intHistogramDataPointSlice = &slicePtrStruct{
element: intHistogramDataPoint,
}

var intHistogramDataPoint = &messagePtrStruct{
var intHistogramDataPoint = &messageValueStruct{
structName: "IntHistogramDataPoint",
description: "// IntHistogramDataPoint is a single data point in a timeseries that describes the time-varying values of a Histogram of int values.",
originFullName: "otlpmetrics.IntHistogramDataPoint",
Expand All @@ -286,7 +286,7 @@ var doubleHistogramDataPointSlice = &slicePtrStruct{
element: doubleHistogramDataPoint,
}

var doubleHistogramDataPoint = &messagePtrStruct{
var doubleHistogramDataPoint = &messageValueStruct{
structName: "DoubleHistogramDataPoint",
description: "// DoubleHistogramDataPoint is a single data point in a timeseries that describes the time-varying values of a Histogram of double values.",
originFullName: "otlpmetrics.DoubleHistogramDataPoint",
Expand All @@ -307,7 +307,7 @@ var doubleSummaryDataPointSlice = &slicePtrStruct{
element: doubleSummaryDataPoint,
}

var doubleSummaryDataPoint = &messagePtrStruct{
var doubleSummaryDataPoint = &messageValueStruct{
structName: "DoubleSummaryDataPoint",
description: "// DoubleSummaryDataPoint is a single data point in a timeseries that describes the time-varying values of a Summary of double values.",
originFullName: "otlpmetrics.DoubleSummaryDataPoint",
Expand All @@ -330,7 +330,7 @@ var quantileValuesSlice = &slicePtrStruct{
element: quantileValues,
}

var quantileValues = &messagePtrStruct{
var quantileValues = &messageValueStruct{
structName: "ValueAtQuantile",
description: "// ValueAtQuantile is a quantile value within a Summary data point",
originFullName: "otlpmetrics.DoubleSummaryDataPoint_ValueAtQuantile",
Expand All @@ -345,7 +345,7 @@ var intExemplarSlice = &slicePtrStruct{
element: intExemplar,
}

var intExemplar = &messagePtrStruct{
var intExemplar = &messageValueStruct{
structName: "IntExemplar",
description: "// IntExemplar is a sample input int measurement.\n//\n" +
"// Exemplars also hold information about the environment when the measurement was recorded,\n" +
Expand All @@ -368,7 +368,7 @@ var doubleExemplarSlice = &slicePtrStruct{
element: doubleExemplar,
}

var doubleExemplar = &messagePtrStruct{
var doubleExemplar = &messageValueStruct{
structName: "DoubleExemplar",
description: "// DoubleExemplar is a sample input double measurement.\n//\n" +
"// Exemplars also hold information about the environment when the measurement was recorded,\n" +
Expand Down
10 changes: 5 additions & 5 deletions cmd/pdatagen/internal/trace_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var resourceSpansSlice = &slicePtrStruct{
element: resourceSpans,
}

var resourceSpans = &messagePtrStruct{
var resourceSpans = &messageValueStruct{
structName: "ResourceSpans",
description: "// InstrumentationLibrarySpans is a collection of spans from a LibraryInstrumentation.",
originFullName: "otlptrace.ResourceSpans",
Expand All @@ -66,7 +66,7 @@ var instrumentationLibrarySpansSlice = &slicePtrStruct{
element: instrumentationLibrarySpans,
}

var instrumentationLibrarySpans = &messagePtrStruct{
var instrumentationLibrarySpans = &messageValueStruct{
structName: "InstrumentationLibrarySpans",
description: "// InstrumentationLibrarySpans is a collection of spans from a LibraryInstrumentation.",
originFullName: "otlptrace.InstrumentationLibrarySpans",
Expand All @@ -85,7 +85,7 @@ var spanSlice = &slicePtrStruct{
element: span,
}

var span = &messagePtrStruct{
var span = &messageValueStruct{
structName: "Span",
description: "// Span represents a single operation within a trace.\n" +
"// See Span definition in OTLP: https://github.com/open-telemetry/opentelemetry-proto/blob/master/opentelemetry/proto/trace/v1/trace.proto#L37",
Expand Down Expand Up @@ -145,7 +145,7 @@ var spanEventSlice = &slicePtrStruct{
element: spanEvent,
}

var spanEvent = &messagePtrStruct{
var spanEvent = &messageValueStruct{
structName: "SpanEvent",
description: "// SpanEvent is a time-stamped annotation of the span, consisting of user-supplied\n" +
"// text description and key-value pairs. See OTLP for event definition.",
Expand All @@ -163,7 +163,7 @@ var spanLinkSlice = &slicePtrStruct{
element: spanLink,
}

var spanLink = &messagePtrStruct{
var spanLink = &messageValueStruct{
structName: "SpanLink",
description: "// SpanLink is a pointer from the current span to another span in the same trace or in a\n" +
"// different trace. See OTLP for link definition.",
Expand Down
7 changes: 1 addition & 6 deletions consumer/pdata/generated_common_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit d2aa6cd

Please sign in to comment.