Skip to content

Commit

Permalink
Expose non-nullable elements from slices of 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 30, 2020
1 parent 7c28105 commit 81c3b9e
Show file tree
Hide file tree
Showing 79 changed files with 497 additions and 2,135 deletions.
1 change: 0 additions & 1 deletion cmd/mdatagen/metrics.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ var Metrics = &metricStruct{
"{{ $name }}",
func() pdata.Metric {
metric := pdata.NewMetric()
metric.InitEmpty()
metric.SetName("{{ $name }}")
metric.SetDescription("{{ $metric.Description }}")
metric.SetUnit("{{ $metric.Unit }}")
Expand Down
2 changes: 1 addition & 1 deletion cmd/pdatagen/internal/base_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,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)
32 changes: 13 additions & 19 deletions cmd/pdatagen/internal/base_slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,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 @@ -80,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 @@ -130,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 @@ -141,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 @@ -194,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 @@ -215,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 @@ -236,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 @@ -474,13 +469,12 @@ func Test${structName}_Resize(t *testing.T) {
func Test${structName}_Append(t *testing.T) {
es := generateTest${structName}()
emptyVal := New${elementName}()
emptyVal := New${elementName}()
es.Append(emptyVal)
assert.EqualValues(t, *(es.At(7)).orig, *emptyVal.orig)
emptyVal2 := New${elementName}()
es.Append(emptyVal2)
assert.EqualValues(t, *(es.At(8)).orig, *emptyVal2.orig)
Expand All @@ -507,7 +501,7 @@ type baseSlice interface {
// Will generate code only for a slice of pointer fields.
type sliceOfPtrs struct {
structName string
element *messagePtrStruct
element *messageValueStruct
}

func (ss *sliceOfPtrs) 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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
element: attributeKeyValue,
}

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

var instrumentationLibraryField = &messageValueField{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 = &sliceOfPtrs{
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 @@ -48,7 +48,7 @@ var resourceSpansSlice = &sliceOfPtrs{
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 @@ -67,7 +67,7 @@ var instrumentationLibrarySpansSlice = &sliceOfPtrs{
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 @@ -86,7 +86,7 @@ var spanSlice = &sliceOfPtrs{
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 @@ -146,7 +146,7 @@ var spanEventSlice = &sliceOfPtrs{
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 @@ -164,7 +164,7 @@ var spanLinkSlice = &sliceOfPtrs{
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
3 changes: 1 addition & 2 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 81c3b9e

Please sign in to comment.