Skip to content

Commit

Permalink
[pdata] Do not wrap orig fields when it's not required (#7081)
Browse files Browse the repository at this point in the history
This change simplifies the generated pdata code to not wrap orig fields in the internal package for structs that are not being used by other packages.

The code generator is adjusted to generate wrapped or unwrapped code for only for structs that need it based on the package name. The only exception is `Slice` struct that was pulled from the generator because:
- We don't have and don't expect to have any new slices that are used by other packages.
- The `Slice` struct have two additional methods `AsRaw` and `FromRaw` that are not generated and defined in a separate file which is a bit confusing.
  • Loading branch information
dmitryax authored Jan 30, 2023
1 parent 6896a37 commit de4867f
Show file tree
Hide file tree
Showing 40 changed files with 2,461 additions and 3,259 deletions.
143 changes: 109 additions & 34 deletions pdata/internal/cmd/pdatagen/internal/base_fields.go

Large diffs are not rendered by default.

232 changes: 89 additions & 143 deletions pdata/internal/cmd/pdatagen/internal/base_slices.go

Large diffs are not rendered by default.

146 changes: 82 additions & 64 deletions pdata/internal/cmd/pdatagen/internal/base_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,10 @@ const messageValueTemplate = `${description}
//
// Must use New${structName} function to create new instances.
// Important: zero-initialized instance is not valid for use.
type ${structName} internal.${internalStructName}
${typeDeclaration}
func new${structName}(orig *${originName}) ${structName} {
return ${structName}(internal.New${internalStructName}(orig))
}
func (ms ${structName}) getOrig() *${originName} {
return internal.GetOrig${internalStructName}(internal.${internalStructName}(ms))
return ${newFuncValue}
}
// New${structName} creates a new empty ${structName}.
Expand All @@ -48,8 +43,12 @@ func New${structName}() ${structName} {
// MoveTo moves all properties from the current struct overriding the destination and
// resetting the current instance to its zero value
func (ms ${structName}) MoveTo(dest ${structName}) {
*dest.getOrig() = *ms.getOrig()
*ms.getOrig() = ${originName}{}
*dest.${origAccessor} = *ms.${origAccessor}
*ms.${origAccessor} = ${originName}{}
}`

const messageValueGetOrigTemplate = `func (ms ${structName}) getOrig() *${originName} {
return internal.GetOrig${structName}(internal.${structName}(ms))
}`

const messageValueCopyToHeaderTemplate = `// CopyTo copies all properties from the current struct overriding the destination.
Expand All @@ -59,82 +58,74 @@ const messageValueCopyToFooterTemplate = `}`

const messageValueTestTemplate = `
func Test${structName}_MoveTo(t *testing.T) {
ms := ${structName}(internal.GenerateTest${internalStructName}())
ms := ${generateTestData}
dest := New${structName}()
ms.MoveTo(dest)
assert.Equal(t, New${structName}(), ms)
assert.Equal(t, ${structName}(internal.GenerateTest${internalStructName}()), dest)
assert.Equal(t, ${generateTestData}, dest)
}
func Test${structName}_CopyTo(t *testing.T) {
ms := New${structName}()
orig := New${structName}()
orig.CopyTo(ms)
assert.Equal(t, orig, ms)
orig = ${structName}(internal.GenerateTest${internalStructName}())
orig = ${generateTestData}
orig.CopyTo(ms)
assert.Equal(t, orig, ms)
}`

const messageValueGenerateTestTemplate = `func GenerateTest${internalStructName}() ${internalStructName} {
orig := ${originName}{}
tv := New${internalStructName}(&orig)
FillTest${internalStructName}(tv)
const messageValueGenerateTestTemplate = `func generateTest${structName}() ${structName} {
tv := New${structName}()
fillTest${structName}(tv)
return tv
}`

const messageValueFillTestHeaderTemplate = `func FillTest${internalStructName}(tv ${internalStructName}) {`
const messageValueFillTestFooterTemplate = `}`
const messageValueGenerateTestTemplateCommon = `func GenerateTest${structName}() ${structName} {
orig := ${originName}{}
tv := New${structName}(&orig)
FillTest${structName}(tv)
return tv
}`

const messageValueAliasTemplate = `
type ${internalStructName} struct {
type ${structName} struct {
orig *${originName}
}
func GetOrig${internalStructName}(ms ${internalStructName}) *${originName} {
func GetOrig${structName}(ms ${structName}) *${originName} {
return ms.orig
}
func New${internalStructName}(orig *${originName}) ${internalStructName} {
return ${internalStructName}{orig: orig}
func New${structName}(orig *${originName}) ${structName} {
return ${structName}{orig: orig}
}`

const newLine = "\n"

type baseStruct interface {
getName() string

getPackageName() string

generateStruct(sb *bytes.Buffer)

generateTests(sb *bytes.Buffer)

generateTestValueHelpers(sb *bytes.Buffer)

generateInternal(sb *bytes.Buffer)
}

// messageValueStruct generates a struct for a proto message. The struct can be generated both as a common struct
// that can be used as a field in struct from other packages and as an isolated struct with depending on a package name.
type messageValueStruct struct {
structName string
internalStructName string
packageName string
description string
originFullName string
fields []baseField
structName string
packageName string
description string
originFullName string
fields []baseField
}

func (ms *messageValueStruct) getName() string {
return ms.structName
}

func (ms *messageValueStruct) getInternalName() string {
if ms.internalStructName != "" {
return ms.internalStructName
}
return ms.structName
}

func (ms *messageValueStruct) getPackageName() string {
return ms.packageName
}
Expand All @@ -144,16 +135,39 @@ func (ms *messageValueStruct) generateStruct(sb *bytes.Buffer) {
switch name {
case "structName":
return ms.structName
case "internalStructName":
return ms.getInternalName()
case "originName":
return ms.originFullName
case "description":
return ms.description
case "typeDeclaration":
if usedByOtherDataTypes(ms.packageName) {
return "type " + ms.structName + " internal." + ms.structName
}
return "type " + ms.structName + " struct {\n\torig *" + ms.originFullName + "\n}"
case "newFuncValue":
if usedByOtherDataTypes(ms.packageName) {
return ms.structName + "(internal.New" + ms.structName + "(orig))"
}
return ms.structName + "{orig}"
case "origAccessor":
return origAccessor(ms)
default:
panic(name)
}
}))
if usedByOtherDataTypes(ms.packageName) {
sb.WriteString(newLine + newLine)
sb.WriteString(os.Expand(messageValueGetOrigTemplate, func(name string) string {
switch name {
case "structName":
return ms.structName
case "originName":
return ms.originFullName
default:
panic(name)
}
}))
}
// Write accessors for the struct
for _, f := range ms.fields {
sb.WriteString(newLine + newLine)
Expand Down Expand Up @@ -184,8 +198,11 @@ func (ms *messageValueStruct) generateTests(sb *bytes.Buffer) {
switch name {
case "structName":
return ms.structName
case "internalStructName":
return ms.getInternalName()
case "generateTestData":
if usedByOtherDataTypes(ms.packageName) {
return ms.structName + "(internal.GenerateTest" + ms.structName + "())"
}
return "generateTest" + ms.structName + "()"
default:
panic(name)
}
Expand All @@ -199,41 +216,42 @@ func (ms *messageValueStruct) generateTests(sb *bytes.Buffer) {
}

func (ms *messageValueStruct) generateTestValueHelpers(sb *bytes.Buffer) {
sb.WriteString(os.Expand(messageValueGenerateTestTemplate, func(name string) string {
// Write generateTest func for the struct
template := messageValueGenerateTestTemplate
if usedByOtherDataTypes(ms.packageName) {
template = messageValueGenerateTestTemplateCommon
}
sb.WriteString(os.Expand(template, func(name string) string {
switch name {
case "internalStructName":
return ms.getInternalName()
case "structName":
return ms.structName
case "originName":
return ms.originFullName
default:
panic(name)
}
}))
sb.WriteString(newLine + newLine)
sb.WriteString(os.Expand(messageValueFillTestHeaderTemplate, func(name string) string {
switch name {
case "internalStructName":
return ms.getInternalName()
default:
panic(name)
}
}))
// Write accessors test value for the struct

// Write fillTest func for the struct
sb.WriteString(newLine + newLine + "func ")
if usedByOtherDataTypes(ms.packageName) {
sb.WriteString("FillTest")
} else {
sb.WriteString("fillTest")
}
sb.WriteString(ms.structName + "(tv " + ms.structName + ") {")
for _, f := range ms.fields {
sb.WriteString(newLine)
f.generateSetWithTestValue(sb)
f.generateSetWithTestValue(ms, sb)
}
sb.WriteString(newLine)
sb.WriteString(os.Expand(messageValueFillTestFooterTemplate, func(name string) string {
panic(name)
}))
sb.WriteString(newLine + "}" + newLine)
}

func (ms *messageValueStruct) generateInternal(sb *bytes.Buffer) {
sb.WriteString(os.Expand(messageValueAliasTemplate, func(name string) string {
switch name {
case "internalStructName":
return ms.getInternalName()
case "structName":
return ms.structName
case "originName":
return ms.originFullName
default:
Expand Down
14 changes: 14 additions & 0 deletions pdata/internal/cmd/pdatagen/internal/files.go

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

12 changes: 0 additions & 12 deletions pdata/internal/cmd/pdatagen/internal/pcommon_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ var commonFile = &File{
},
structs: []baseStruct{
scope,
attributeValueSlice,
},
}

Expand All @@ -58,12 +57,6 @@ var scope = &messageValueStruct{
var mapStruct = &sliceOfPtrs{
structName: "Map",
packageName: "pcommon",
element: attributeKeyValue,
}

var attributeKeyValue = &messageValueStruct{
structName: "Map",
packageName: "pcommon",
}

var scopeField = &messageValueField{
Expand Down Expand Up @@ -120,11 +113,6 @@ var anyValue = &messageValueStruct{
originFullName: "otlpcommon.AnyValue",
}

var attributeValueSlice = &sliceOfValues{
structName: "Slice",
element: anyValue,
}

var traceIDField = &primitiveTypedField{
fieldName: "TraceID",
originFieldName: "TraceId",
Expand Down
7 changes: 3 additions & 4 deletions pdata/internal/cmd/pdatagen/internal/plogotlp_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ var logOtlpFile = &File{
}

var exportLogsPartialSuccess = &messageValueStruct{
structName: "ExportPartialSuccess",
internalStructName: "LogsExportPartialSuccess",
description: "// ExportPartialSuccess represents the details of a partially successful export request.",
originFullName: "otlpcollectorlog.ExportLogsPartialSuccess",
structName: "ExportPartialSuccess",
description: "// ExportPartialSuccess represents the details of a partially successful export request.",
originFullName: "otlpcollectorlog.ExportLogsPartialSuccess",
fields: []baseField{
&primitiveField{
fieldName: "RejectedLogRecords",
Expand Down
7 changes: 3 additions & 4 deletions pdata/internal/cmd/pdatagen/internal/pmetricotlp_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ var metricsOtlpFile = &File{
}

var exportMetricsPartialSuccess = &messageValueStruct{
structName: "ExportPartialSuccess",
internalStructName: "MetricsExportPartialSuccess",
description: "// ExportPartialSuccess represents the details of a partially successful export request.",
originFullName: "otlpcollectormetrics.ExportMetricsPartialSuccess",
structName: "ExportPartialSuccess",
description: "// ExportPartialSuccess represents the details of a partially successful export request.",
originFullName: "otlpcollectormetrics.ExportMetricsPartialSuccess",
fields: []baseField{
&primitiveField{
fieldName: "RejectedDataPoints",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ func New${structName}(orig *[]${itemType}) ${structName} {
return ${structName}{orig: orig}
}`

// primitiveSliceStruct generates a struct for a slice of primitive value elements. The structs are always generated
// in a way that they can be used as fields in structs from other packages (using the internal package).
type primitiveSliceStruct struct {
structName string
packageName string
Expand Down
7 changes: 3 additions & 4 deletions pdata/internal/cmd/pdatagen/internal/ptraceotlp_structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ var traceOtlpFile = &File{
}

var exportTracePartialSuccess = &messageValueStruct{
structName: "ExportPartialSuccess",
internalStructName: "TracesExportPartialSuccess",
description: "// ExportPartialSuccess represents the details of a partially successful export request.",
originFullName: "otlpcollectortrace.ExportTracePartialSuccess",
structName: "ExportPartialSuccess",
description: "// ExportPartialSuccess represents the details of a partially successful export request.",
originFullName: "otlpcollectortrace.ExportTracePartialSuccess",
fields: []baseField{
&primitiveField{
fieldName: "RejectedSpans",
Expand Down
Loading

0 comments on commit de4867f

Please sign in to comment.