[chore][cmd/mdatagen] Require all fields to have a non-empty description

mx-psi · mx-psi · commit 1692a18d7165 · 2026-07-24T16:34:54.000+02:00
diff --git a/cmd/mdatagen/internal/command.go b/cmd/mdatagen/internal/command.go
@@ -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
diff --git a/cmd/mdatagen/internal/command_test.go b/cmd/mdatagen/internal/command_test.go
@@ -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."},
 					},
 				},
 			},
@@ -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."},
 					},
 				},
 			},
@@ -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{}
diff --git a/cmd/mdatagen/internal/loader_test.go b/cmd/mdatagen/internal/loader_test.go
@@ -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.",
diff --git a/cmd/mdatagen/internal/samplereceiver/README.md b/cmd/mdatagen/internal/samplereceiver/README.md
@@ -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
diff --git a/cmd/mdatagen/internal/samplereceiver/config.schema.json b/cmd/mdatagen/internal/samplereceiver/config.schema.json
@@ -43,6 +43,7 @@
           "maximum": 10000
         }
       },
+      "description": "Configuration imported from the sample shared package.",
       "required": [
         "host_name"
       ]
diff --git a/cmd/mdatagen/internal/samplereceiver/generated_config.go b/cmd/mdatagen/internal/samplereceiver/generated_config.go
diff --git a/cmd/mdatagen/internal/samplereceiver/metadata.yaml b/cmd/mdatagen/internal/samplereceiver/metadata.yaml
@@ -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]
 
diff --git a/cmd/mdatagen/internal/samplescraper/config.schema.json b/cmd/mdatagen/internal/samplescraper/config.schema.json
@@ -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))+$"
           },
diff --git a/cmd/mdatagen/internal/samplescraper/generated_config.go b/cmd/mdatagen/internal/samplescraper/generated_config.go
diff --git a/cmd/mdatagen/internal/samplescraper/metadata.yaml b/cmd/mdatagen/internal/samplescraper/metadata.yaml
@@ -61,6 +61,7 @@ config:
         type: object
         properties:
           interval:
+            description: Scrape interval for this target.
             type: duration
             x-optional: true
             default: 10s
diff --git a/config/configcompression/config.schema.json b/config/configcompression/config.schema.json
@@ -6,7 +6,8 @@
       "type": "object",
       "properties": {
         "level": {
-          "type": "integer"
+          "type": "integer",
+          "description": "Level of compression."
         }
       }
     },
diff --git a/config/configcompression/generated_config.go b/config/configcompression/generated_config.go
diff --git a/config/configcompression/metadata.yaml b/config/configcompression/metadata.yaml
@@ -11,6 +11,7 @@ exported_configs:
     type: object
     properties:
       level:
+        description: Level of compression.
         $ref: level
   level:
     type: integer
diff --git a/internal/schemagen/model.go b/internal/schemagen/model.go
@@ -6,6 +6,8 @@ package schemagen // import "go.opentelemetry.io/collector/internal/schemagen"
 import (
 	"errors"
 	"fmt"
+	"sort"
+	"strings"
 
 	"go.opentelemetry.io/collector/confmap"
 )
@@ -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.
diff --git a/internal/schemagen/model_test.go b/internal/schemagen/model_test.go
@@ -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())
+		})
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,7 @@`
`43`	`43`	`"maximum": 10000`
`44`	`44`	`}`
`45`	`45`	`},`
	`46`	`+ "description": "Configuration imported from the sample shared package.",`
`46`	`47`	`"required": [`
`47`	`48`	`"host_name"`
`48`	`49`	`]`