Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/mdatagen/internal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,11 @@ func generateConfigFiles(md Metadata, mdDir, importRootPath string) error {
return fmt.Errorf("failed to resolve config schema: %w", err)
}

if missing := resolvedSchema.CollectMissingDescriptions(); len(missing) > 0 {
return fmt.Errorf("one or more config field(s) missing a description:\n\t%s",
strings.Join(missing, "\n\t"))
}

// do a shallow copy of Metadata and replace Config with resolved schema
mdWithConfig := md
mdWithConfig.ConfigsMetadata = resolvedSchema
Expand Down
36 changes: 33 additions & 3 deletions cmd/mdatagen/internal/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ func TestGenerateConfigFiles_ExportedConfigsWithoutConfig(t *testing.T) {
"sample_config": {
Type: "object",
Properties: map[string]*cfggen.ConfigMetadata{
"endpoint": {Type: "string"},
"endpoint": {Type: "string", Description: "The endpoint to connect to."},
},
},
},
Expand Down Expand Up @@ -635,14 +635,14 @@ func TestGenerateConfigFiles_ExportedConfigsWithConfig(t *testing.T) {
Config: &cfggen.ConfigMetadata{
Type: "object",
Properties: map[string]*cfggen.ConfigMetadata{
"endpoint": {Type: "string"},
"endpoint": {Type: "string", Description: "The endpoint to connect to."},
},
},
ExportedConfigs: map[string]*cfggen.ConfigMetadata{
"sample_config": {
Type: "object",
Properties: map[string]*cfggen.ConfigMetadata{
"host_name": {Type: "string"},
"host_name": {Type: "string", Description: "Host name to connect to."},
},
},
},
Expand All @@ -664,6 +664,36 @@ func TestGenerateConfigFiles_ExportedConfigsWithConfig(t *testing.T) {
require.Contains(t, string(generatedConfig), "type SampleConfig struct")
}

func TestGenerateConfigFiles_MissingDescriptionFails(t *testing.T) {
root := t.TempDir()
tmpdir := filepath.Join(root, "shortname")
require.NoError(t, os.MkdirAll(tmpdir, 0o700))
require.NoError(t, os.WriteFile(filepath.Join(root, "go.mod"), []byte("module testmodule\n"), 0o600))

md := Metadata{
Type: "test",
PackageName: "testmodule/shortname",
Status: &Status{Class: "receiver"},
ConfigsMetadata: &cfggen.ConfigsMetadata{
Config: &cfggen.ConfigMetadata{
Type: "object",
Properties: map[string]*cfggen.ConfigMetadata{
"endpoint": {Type: "string", Description: "The endpoint to connect to."},
"timeout": {Type: "duration"},
},
},
},
}

err := generateConfigFiles(md, tmpdir, "testmodule")
require.Error(t, err)
require.Contains(t, err.Error(), "config.timeout")
require.NotContains(t, err.Error(), "config.endpoint")

// No files should have been generated when validation fails.
require.NoFileExists(t, filepath.Join(tmpdir, "generated_config.go"))
}

func TestInjectInternalMetadataDefs(t *testing.T) {
t.Run("skips when metadata has no internal definitions", func(t *testing.T) {
src := &cfggen.ConfigsMetadata{}
Expand Down
3 changes: 2 additions & 1 deletion cmd/mdatagen/internal/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ func TestLoadMetadata(t *testing.T) {
Default: "localhost:12345",
},
"sample_pkg": {
Ref: "../samplepkg.sample_config",
Description: "Configuration imported from the sample shared package.",
Ref: "../samplepkg.sample_config",
},
"timeout": {
Description: "Timeout for scraping metrics.",
Expand Down
2 changes: 1 addition & 1 deletion cmd/mdatagen/internal/samplereceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ This is where warnings are described.
| `max_results` | int | 100 | no | Maximum number of results to return per scrape. |
| `metrics` | object (see [metrics](#metrics)) | | no | MetricsConfig provides config for sample metrics. |
| `resource_attributes` | object (see [resource_attributes](#resource_attributes)) | | no | ResourceAttributesConfig provides config for sample resource attributes. |
| `sample_pkg` | object (see [sample_pkg](#sample_pkg)) | | no | |
| `sample_pkg` | object (see [sample_pkg](#sample_pkg)) | | no | Configuration imported from the sample shared package. |
| `timeout` | duration | 10s | no | Timeout for scraping metrics. |

### <a id="metrics"></a>metrics
Expand Down
1 change: 1 addition & 0 deletions cmd/mdatagen/internal/samplereceiver/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"maximum": 10000
}
},
"description": "Configuration imported from the sample shared package.",
"required": [
"host_name"
]
Expand Down
5 changes: 3 additions & 2 deletions cmd/mdatagen/internal/samplereceiver/generated_config.go

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

1 change: 1 addition & 0 deletions cmd/mdatagen/internal/samplereceiver/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ config:
description: Extra HTTP headers to attach to each request.
type: opaque_map
sample_pkg:
description: Configuration imported from the sample shared package.
$ref: ../samplepkg.sample_config
required: [endpoint]

Expand Down
1 change: 1 addition & 0 deletions cmd/mdatagen/internal/samplescraper/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"properties": {
"interval": {
"type": "string",
"description": "Scrape interval for this target.",
"default": "10s",
"pattern": "^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
},
Expand Down
1 change: 1 addition & 0 deletions cmd/mdatagen/internal/samplescraper/generated_config.go

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

1 change: 1 addition & 0 deletions cmd/mdatagen/internal/samplescraper/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ config:
type: object
properties:
interval:
description: Scrape interval for this target.
type: duration
x-optional: true
default: 10s
Expand Down
3 changes: 2 additions & 1 deletion config/configcompression/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"type": "object",
"properties": {
"level": {
"type": "integer"
"type": "integer",
"description": "Level of compression."
}
}
},
Expand Down
1 change: 1 addition & 0 deletions config/configcompression/generated_config.go

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

1 change: 1 addition & 0 deletions config/configcompression/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ exported_configs:
type: object
properties:
level:
description: Level of compression.
$ref: level
level:
type: integer
Expand Down
51 changes: 51 additions & 0 deletions internal/schemagen/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package schemagen // import "go.opentelemetry.io/collector/internal/schemagen"
import (
"errors"
"fmt"
"sort"
"strings"

"go.opentelemetry.io/collector/confmap"
)
Expand Down Expand Up @@ -99,6 +101,55 @@ func (md *ConfigsMetadata) Validate() error {
return nil
}

// CollectMissingDescriptions returns the config fields without a description.
func (md *ConfigsMetadata) CollectMissingDescriptions() []string {
var missing []string
if md.Config != nil {
collectMissingDescriptions(md.Config, "config", &missing)
}
for _, name := range sortedConfigKeys(md.ExportedConfigs) {
cfg := md.ExportedConfigs[name]
if cfg == nil || cfg.InternalOnly {
continue
}
collectMissingDescriptions(cfg, "exported_configs."+name, &missing)
}
sort.Strings(missing)
return missing
}

func collectMissingDescriptions(md *ConfigMetadata, path string, missing *[]string) {
for _, name := range sortedConfigKeys(md.Properties) {
prop := md.Properties[name]
if prop == nil {
continue
}
fieldPath := path + "." + name
if strings.TrimSpace(prop.Description) == "" {
*missing = append(*missing, fieldPath)
}
// Ignore external types.
if prop.Ref == "" {
collectMissingDescriptions(prop, fieldPath, missing)
}
}
if md.Items != nil && md.Items.Ref == "" {
collectMissingDescriptions(md.Items, path+".items", missing)
}
if md.AdditionalProperties != nil && md.AdditionalProperties.Ref == "" {
collectMissingDescriptions(md.AdditionalProperties, path+".additionalProperties", missing)
}
}

func sortedConfigKeys(m map[string]*ConfigMetadata) []string {
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
sort.Strings(keys)
return keys
}

// MergeFrom copies any field from other that is not already set on md.
// For maps (Properties, PatternProperties), missing keys are merged in individually.
// Calling MergeFrom on a zero-value ConfigMetadata is equivalent to a deep clone of other.
Expand Down
100 changes: 100 additions & 0 deletions internal/schemagen/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -647,3 +647,103 @@ func TestConfigMetadata_MergeFrom(t *testing.T) {
assert.Equal(t, "Field", md.GoStruct.FieldName)
})
}

func TestConfigsMetadata_CollectMissingDescriptions(t *testing.T) {
tests := []struct {
name string
md *ConfigsMetadata
want []string
}{
{
name: "nil config",
md: &ConfigsMetadata{},
want: nil,
},
{
name: "all described",
md: &ConfigsMetadata{
Config: &ConfigMetadata{Properties: map[string]*ConfigMetadata{
"endpoint": {Description: "The endpoint.", Type: "string"},
}},
},
want: nil,
},
{
name: "missing on config property",
md: &ConfigsMetadata{
Config: &ConfigMetadata{Properties: map[string]*ConfigMetadata{
"endpoint": {Description: "The endpoint.", Type: "string"},
"timeout": {Type: "duration"},
}},
},
want: []string{"config.timeout"},
},
{
name: "whitespace-only description counts as missing",
md: &ConfigsMetadata{
Config: &ConfigMetadata{Properties: map[string]*ConfigMetadata{
"endpoint": {Description: " ", Type: "string"},
}},
},
want: []string{"config.endpoint"},
},
{
name: "ref property requires its own description but is not recursed into",
md: &ConfigsMetadata{
Config: &ConfigMetadata{Properties: map[string]*ConfigMetadata{
// Field has no description, so it is reported. The subtree
// merged from the ref must not be walked (would misattribute).
"tls": {Ref: "../configtls.client_config", Properties: map[string]*ConfigMetadata{
"insecure": {Type: "bool"},
}},
}},
},
want: []string{"config.tls"},
},
{
name: "nested inline object is recursed",
md: &ConfigsMetadata{
Config: &ConfigMetadata{Properties: map[string]*ConfigMetadata{
"auth": {Description: "Auth settings.", Type: "object", Properties: map[string]*ConfigMetadata{
"token": {Type: "string"},
}},
}},
},
want: []string{"config.auth.token"},
},
{
name: "array items are recursed",
md: &ConfigsMetadata{
Config: &ConfigMetadata{Properties: map[string]*ConfigMetadata{
"targets": {Description: "Targets.", Type: "array", Items: &ConfigMetadata{
Type: "object", Properties: map[string]*ConfigMetadata{
"interval": {Type: "duration"},
},
}},
}},
},
want: []string{"config.targets.items.interval"},
},
{
name: "internal-only exported configs are skipped, non-internal reported, sorted",
md: &ConfigsMetadata{
ExportedConfigs: map[string]*ConfigMetadata{
"MetricsConfig": {InternalOnly: true, Properties: map[string]*ConfigMetadata{
"enabled": {Type: "bool"},
}},
"ClientConfig": {Properties: map[string]*ConfigMetadata{
"timeout": {Type: "duration"},
"endpoint": {Type: "string"},
}},
},
},
want: []string{"exported_configs.ClientConfig.endpoint", "exported_configs.ClientConfig.timeout"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.md.CollectMissingDescriptions())
})
}
}
Loading