From 04bb6d962079450f7867ee1e854ecb1d75f37d83 Mon Sep 17 00:00:00 2001 From: Dmitrii Anoshin Date: Tue, 30 Sep 2025 12:17:02 -0700 Subject: [PATCH] [mdatagen] Improve validation for attribute `enabled` field Resource attributes now require an explicit `enabled` field in metadata.yaml files, while regular attributes are prohibited from having this field. This improves validation and prevents configuration errors. --- .chloggen/control-attribute-enabled.yaml | 27 ++++++++++++++++++++++++ cmd/mdatagen/internal/loader_test.go | 20 +++++++++++------- cmd/mdatagen/internal/metadata.go | 22 +++++++++++++++++-- 3 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 .chloggen/control-attribute-enabled.yaml diff --git a/.chloggen/control-attribute-enabled.yaml b/.chloggen/control-attribute-enabled.yaml new file mode 100644 index 00000000000..e394d1b059f --- /dev/null +++ b/.chloggen/control-attribute-enabled.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: mdatagen + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Improve validation for resource attribute `enabled` field in metadata files + +# One or more tracking issues or pull requests related to the change +issues: [12722] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + Resource attributes now require an explicit `enabled` field in metadata.yaml files, while regular attributes + are prohibited from having this field. This improves validation and prevents configuration errors. + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/cmd/mdatagen/internal/loader_test.go b/cmd/mdatagen/internal/loader_test.go index 4ea52e111d9..7ab082a8f91 100644 --- a/cmd/mdatagen/internal/loader_test.go +++ b/cmd/mdatagen/internal/loader_test.go @@ -15,6 +15,10 @@ import ( "go.opentelemetry.io/collector/pdata/pmetric" ) +func boolPtr(b bool) *bool { + return &b +} + func TestTwoPackagesInDirectory(t *testing.T) { contents, err := os.ReadFile("testdata/twopackages.yaml") require.NoError(t, err) @@ -70,7 +74,7 @@ func TestLoadMetadata(t *testing.T) { ResourceAttributes: map[AttributeName]Attribute{ "string.resource.attr": { Description: "Resource attribute with any string value.", - Enabled: true, + EnabledPtr: boolPtr(true), Type: ValueType{ ValueType: pcommon.ValueTypeStr, }, @@ -78,7 +82,7 @@ func TestLoadMetadata(t *testing.T) { }, "string.enum.resource.attr": { Description: "Resource attribute with a known set of string values.", - Enabled: true, + EnabledPtr: boolPtr(true), Enum: []string{"one", "two"}, Type: ValueType{ ValueType: pcommon.ValueTypeStr, @@ -87,7 +91,7 @@ func TestLoadMetadata(t *testing.T) { }, "optional.resource.attr": { Description: "Explicitly disabled ResourceAttribute.", - Enabled: false, + EnabledPtr: boolPtr(false), Type: ValueType{ ValueType: pcommon.ValueTypeStr, }, @@ -95,7 +99,7 @@ func TestLoadMetadata(t *testing.T) { }, "slice.resource.attr": { Description: "Resource attribute with a slice value.", - Enabled: true, + EnabledPtr: boolPtr(true), Type: ValueType{ ValueType: pcommon.ValueTypeSlice, }, @@ -103,7 +107,7 @@ func TestLoadMetadata(t *testing.T) { }, "map.resource.attr": { Description: "Resource attribute with a map value.", - Enabled: true, + EnabledPtr: boolPtr(true), Type: ValueType{ ValueType: pcommon.ValueTypeMap, }, @@ -114,7 +118,7 @@ func TestLoadMetadata(t *testing.T) { Warnings: Warnings{ IfEnabledNotSet: "This resource_attribute will be disabled by default soon.", }, - Enabled: true, + EnabledPtr: boolPtr(true), Type: ValueType{ ValueType: pcommon.ValueTypeStr, }, @@ -125,7 +129,7 @@ func TestLoadMetadata(t *testing.T) { Warnings: Warnings{ IfConfigured: "This resource_attribute is deprecated and will be removed soon.", }, - Enabled: false, + EnabledPtr: boolPtr(false), Type: ValueType{ ValueType: pcommon.ValueTypeStr, }, @@ -136,7 +140,7 @@ func TestLoadMetadata(t *testing.T) { Warnings: Warnings{ IfEnabled: "This resource_attribute is deprecated and will be removed soon.", }, - Enabled: true, + EnabledPtr: boolPtr(true), Type: ValueType{ ValueType: pcommon.ValueTypeStr, }, diff --git a/cmd/mdatagen/internal/metadata.go b/cmd/mdatagen/internal/metadata.go index 3fe2cbe82c9..2524c974076 100644 --- a/cmd/mdatagen/internal/metadata.go +++ b/cmd/mdatagen/internal/metadata.go @@ -114,6 +114,9 @@ func (md *Metadata) validateResourceAttributes() error { if attr.Type == empty { errs = errors.Join(errs, fmt.Errorf("empty type for resource attribute: %v", name)) } + if attr.EnabledPtr == nil { + errs = errors.Join(errs, fmt.Errorf("enabled field is required for resource attribute: %v", name)) + } } return errs } @@ -140,6 +143,9 @@ func (md *Metadata) validateAttributes(usedAttrs map[AttributeName]bool) error { if attr.Type == empty { errs = errors.Join(errs, fmt.Errorf("empty type for attribute: %v", attrName)) } + if attr.EnabledPtr != nil { + errs = errors.Join(errs, fmt.Errorf("enabled field is not allowed for regular attribute: %v", attrName)) + } if !usedAttrs[attrName] { unusedAttrs = append(unusedAttrs, attrName) } @@ -292,8 +298,8 @@ type Attribute struct { Description string `mapstructure:"description"` // NameOverride can be used to override the attribute name. NameOverride string `mapstructure:"name_override"` - // Enabled defines whether the attribute is enabled by default. - Enabled bool `mapstructure:"enabled"` + // EnabledPtr defines whether the attribute is enabled by default. + EnabledPtr *bool `mapstructure:"enabled"` // Include can be used to filter attributes. Include []filter.Config `mapstructure:"include"` // Include can be used to filter attributes. @@ -310,6 +316,18 @@ type Attribute struct { Optional bool `mapstructure:"optional"` } +// Enabled returns the boolean value of EnabledPtr. +// This method is needed to differentiate between different types of attributes: +// - Resource attributes: EnabledPtr is always set (non-nil) due to validation +// - Regular attributes: EnabledPtr is always nil due to validation +// Panics if EnabledPtr is nil, indicating incorrect template usage. +func (a Attribute) Enabled() bool { + if a.EnabledPtr == nil { + panic("Enabled() must not be called on regular attributes, only on resource attributes") + } + return *a.EnabledPtr +} + // Name returns actual name of the attribute that is set on the metric after applying NameOverride. func (a Attribute) Name() AttributeName { if a.NameOverride != "" {