Skip to content

Commit d4635e4

Browse files
committed
refactor(mdatagen): convert feature gates from map to list
- Changed FeatureGates to []FeatureGate with ID field for preserved ordering - Added duplicate ID validation - Updated templates to iterate list using .ID - Added feature_gates section to schema and README - Updated tests and testdata - Removed accidentally committed mdatagen binary Addresses all feedback from @mx-psi on PR open-telemetry#14130
1 parent 1b485b0 commit d4635e4

File tree

9 files changed

+78
-51
lines changed

9 files changed

+78
-51
lines changed

cmd/mdatagen/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ status:
7272
beta: [metrics, traces]
7373
7474
feature_gates:
75-
mycomponent.newFeature:
75+
- id: mycomponent.newFeature
7676
description: 'Enables new feature functionality that improves performance'
7777
stage: alpha
7878
from_version: 'v0.100.0'
7979
reference_url: 'https://github.com/open-telemetry/opentelemetry-collector/issues/12345'
8080
81-
mycomponent.stableFeature:
81+
- id: mycomponent.stableFeature
8282
description: 'A feature that has reached stability'
8383
stage: stable
8484
from_version: 'v0.90.0'

cmd/mdatagen/internal/metadata.go

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ type Metadata struct {
4949
// PackageName is the name of the package where the component is defined.
5050
PackageName string `mapstructure:"package_name"`
5151
// FeatureGates that are managed by the component.
52-
FeatureGates map[FeatureGateID]FeatureGate `mapstructure:"feature_gates"`
52+
FeatureGates []FeatureGate `mapstructure:"feature_gates"`
53+
// FeatureGates that are managed by the component.
5354
}
5455

5556
func (md Metadata) GetCodeCovComponentID() string {
@@ -278,16 +279,25 @@ func validateEvents(events map[EventName]Event, attributes map[AttributeName]Att
278279

279280
func (md *Metadata) validateFeatureGates() error {
280281
var errs error
281-
for gateID, gate := range md.FeatureGates {
282+
seen := make(map[FeatureGateID]bool)
283+
284+
for i, gate := range md.FeatureGates {
282285
// Validate gate ID is not empty
283-
if string(gateID) == "" {
284-
errs = errors.Join(errs, errors.New("feature gate ID cannot be empty"))
286+
if string(gate.ID) == "" {
287+
errs = errors.Join(errs, fmt.Errorf("feature gate at index %d: ID cannot be empty", i))
288+
continue
289+
}
290+
291+
// Check for duplicate IDs
292+
if seen[gate.ID] {
293+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": duplicate ID`, gate.ID))
285294
continue
286295
}
296+
seen[gate.ID] = true
287297

288298
// Validate gate has required fields
289299
if gate.Description == "" {
290-
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": description is required`, gateID))
300+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": description is required`, gate.ID))
291301
}
292302

293303
// Validate stage is one of the allowed values
@@ -298,20 +308,20 @@ func (md *Metadata) validateFeatureGates() error {
298308
FeatureGateStageDeprecated: true,
299309
}
300310
if !validStages[gate.Stage] {
301-
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": invalid stage "%v", must be one of: alpha, beta, stable, deprecated`, gateID, gate.Stage))
311+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": invalid stage "%v", must be one of: alpha, beta, stable, deprecated`, gate.ID, gate.Stage))
302312
}
303313

304314
// Validate version formats if provided
305315
if gate.FromVersion != "" && !strings.HasPrefix(gate.FromVersion, "v") {
306-
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": from_version "%v" must start with 'v'`, gateID, gate.FromVersion))
316+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": from_version "%v" must start with 'v'`, gate.ID, gate.FromVersion))
307317
}
308318
if gate.ToVersion != "" && !strings.HasPrefix(gate.ToVersion, "v") {
309-
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": to_version "%v" must start with 'v'`, gateID, gate.ToVersion))
319+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": to_version "%v" must start with 'v'`, gate.ID, gate.ToVersion))
310320
}
311321

312322
// Validate that stable/deprecated gates should have to_version
313323
if (gate.Stage == FeatureGateStageStable || gate.Stage == FeatureGateStageDeprecated) && gate.ToVersion == "" {
314-
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": to_version is required for %v stage gates`, gateID, gate.Stage))
324+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": to_version is required for %v stage gates`, gate.ID, gate.Stage))
315325
}
316326
}
317327
return errs
@@ -577,6 +587,8 @@ const (
577587

578588
// FeatureGate represents a feature gate definition in metadata.
579589
type FeatureGate struct {
590+
// ID is the unique identifier for the feature gate.
591+
ID FeatureGateID `mapstructure:"id"`
580592
// Description of the feature gate.
581593
Description string `mapstructure:"description"`
582594
// Stage is the lifecycle stage of the feature gate.

cmd/mdatagen/internal/metadata_test.go

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -328,62 +328,61 @@ func TestValidateFeatureGates(t *testing.T) {
328328
tests := []struct {
329329
name string
330330
featureGate FeatureGate
331-
gateID FeatureGateID
332331
wantErr string
333332
}{
334333
{
335-
name: "valid alpha gate",
336-
gateID: "component.feature",
334+
name: "valid alpha gate",
337335
featureGate: FeatureGate{
336+
ID: "component.feature",
338337
Description: "Test feature gate",
339338
Stage: FeatureGateStageAlpha,
340339
FromVersion: "v0.100.0",
341340
ReferenceURL: "https://example.com",
342341
},
343342
},
344343
{
345-
name: "valid stable gate with to_version",
346-
gateID: "component.stable",
344+
name: "valid stable gate with to_version",
347345
featureGate: FeatureGate{
346+
ID: "component.stable",
348347
Description: "Stable feature gate",
349348
Stage: FeatureGateStageStable,
350349
FromVersion: "v0.90.0",
351350
ToVersion: "v0.95.0",
352351
},
353352
},
354353
{
355-
name: "empty description",
356-
gateID: "component.feature",
354+
name: "empty description",
357355
featureGate: FeatureGate{
356+
ID: "component.feature",
358357
Stage: FeatureGateStageAlpha,
359358
FromVersion: "v0.100.0",
360359
},
361360
wantErr: `description is required`,
362361
},
363362
{
364-
name: "invalid stage",
365-
gateID: "component.feature",
363+
name: "invalid stage",
366364
featureGate: FeatureGate{
365+
ID: "component.feature",
367366
Description: "Test feature",
368367
Stage: "invalid",
369368
FromVersion: "v0.100.0",
370369
},
371370
wantErr: `invalid stage "invalid"`,
372371
},
373372
{
374-
name: "from_version without v prefix",
375-
gateID: "component.feature",
373+
name: "from_version without v prefix",
376374
featureGate: FeatureGate{
375+
ID: "component.feature",
377376
Description: "Test feature",
378377
Stage: FeatureGateStageAlpha,
379378
FromVersion: "0.100.0",
380379
},
381380
wantErr: `from_version "0.100.0" must start with 'v'`,
382381
},
383382
{
384-
name: "to_version without v prefix",
385-
gateID: "component.feature",
383+
name: "to_version without v prefix",
386384
featureGate: FeatureGate{
385+
ID: "component.feature",
387386
Description: "Test feature",
388387
Stage: FeatureGateStageStable,
389388
FromVersion: "v0.90.0",
@@ -392,19 +391,19 @@ func TestValidateFeatureGates(t *testing.T) {
392391
wantErr: `to_version "0.95.0" must start with 'v'`,
393392
},
394393
{
395-
name: "stable gate missing to_version",
396-
gateID: "component.feature",
394+
name: "stable gate missing to_version",
397395
featureGate: FeatureGate{
396+
ID: "component.feature",
398397
Description: "Test feature",
399398
Stage: FeatureGateStageStable,
400399
FromVersion: "v0.90.0",
401400
},
402401
wantErr: `to_version is required for stable stage gates`,
403402
},
404403
{
405-
name: "deprecated gate missing to_version",
406-
gateID: "component.feature",
404+
name: "deprecated gate missing to_version",
407405
featureGate: FeatureGate{
406+
ID: "component.feature",
408407
Description: "Test feature",
409408
Stage: FeatureGateStageDeprecated,
410409
FromVersion: "v0.90.0",
@@ -416,9 +415,7 @@ func TestValidateFeatureGates(t *testing.T) {
416415
for _, tt := range tests {
417416
t.Run(tt.name, func(t *testing.T) {
418417
md := &Metadata{
419-
FeatureGates: map[FeatureGateID]FeatureGate{
420-
tt.gateID: tt.featureGate,
421-
},
418+
FeatureGates: []FeatureGate{tt.featureGate},
422419
}
423420
err := md.validateFeatureGates()
424421
if tt.wantErr != "" {
@@ -433,14 +430,34 @@ func TestValidateFeatureGates(t *testing.T) {
433430

434431
func TestValidateFeatureGatesEmptyID(t *testing.T) {
435432
md := &Metadata{
436-
FeatureGates: map[FeatureGateID]FeatureGate{
437-
"": {
433+
FeatureGates: []FeatureGate{
434+
{
438435
Description: "Test",
439436
Stage: FeatureGateStageAlpha,
440437
},
441438
},
442439
}
443440
err := md.validateFeatureGates()
444441
require.Error(t, err)
445-
assert.ErrorContains(t, err, "feature gate ID cannot be empty")
442+
assert.ErrorContains(t, err, "ID cannot be empty")
443+
}
444+
445+
func TestValidateFeatureGatesDuplicateID(t *testing.T) {
446+
md := &Metadata{
447+
FeatureGates: []FeatureGate{
448+
{
449+
ID: "component.feature",
450+
Description: "Test feature",
451+
Stage: FeatureGateStageAlpha,
452+
},
453+
{
454+
ID: "component.feature",
455+
Description: "Duplicate feature",
456+
Stage: FeatureGateStageAlpha,
457+
},
458+
},
459+
}
460+
err := md.validateFeatureGates()
461+
require.Error(t, err)
462+
assert.ErrorContains(t, err, "duplicate ID")
446463
}

cmd/mdatagen/internal/templates/documentation.md.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,8 @@ This component has the following feature gates:
273273

274274
| Feature Gate | Stage | Description | From Version | To Version | Reference |
275275
| ------------ | ----- | ----------- | ------------ | ---------- | --------- |
276-
{{- range $gateID, $gate := .FeatureGates }}
277-
| `{{ $gateID }}` | {{ $gate.Stage }} | {{ $gate.Description }} | {{ if $gate.FromVersion }}{{ $gate.FromVersion }}{{ else }}N/A{{ end }} | {{ if $gate.ToVersion }}{{ $gate.ToVersion }}{{ else }}N/A{{ end }} | {{ if $gate.ReferenceURL }}[Link]({{ $gate.ReferenceURL }}){{ else }}N/A{{ end }} |
276+
{{- range .FeatureGates }}
277+
| `{{ .ID }}` | {{ .Stage }} | {{ .Description }} | {{ if .FromVersion }}{{ .FromVersion }}{{ else }}N/A{{ end }} | {{ if .ToVersion }}{{ .ToVersion }}{{ else }}N/A{{ end }} | {{ if .ReferenceURL }}[Link]({{ .ReferenceURL }}){{ else }}N/A{{ end }} |
278278
{{- end }}
279279

280280
For more information about feature gates, see the [Feature Gates](https://github.com/open-telemetry/opentelemetry-collector/blob/main/featuregate/README.md) documentation.

cmd/mdatagen/internal/templates/feature_gates.md.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ This component has the following feature gates:
66

77
| Feature Gate | Stage | Description | From Version | To Version | Reference |
88
| ------------ | ----- | ----------- | ------------ | ---------- | --------- |
9-
{{- range $gateID, $gate := .FeatureGates }}
10-
| `{{ $gateID }}` | {{ $gate.Stage }} | {{ $gate.Description }} | {{ if $gate.FromVersion }}{{ $gate.FromVersion }}{{ else }}N/A{{ end }} | {{ if $gate.ToVersion }}{{ $gate.ToVersion }}{{ else }}N/A{{ end }} | {{ if $gate.ReferenceURL }}[Link]({{ $gate.ReferenceURL }}){{ else }}N/A{{ end }} |
9+
{{- range .FeatureGates }}
10+
| `{{ .ID }}` | {{ .Stage }} | {{ .Description }} | {{ if .FromVersion }}{{ .FromVersion }}{{ else }}N/A{{ end }} | {{ if .ToVersion }}{{ .ToVersion }}{{ else }}N/A{{ end }} | {{ if .ReferenceURL }}[Link]({{ .ReferenceURL }}){{ else }}N/A{{ end }} |
1111
{{- end }}
1212

1313
For more information about feature gates, see the [Feature Gates](https://github.com/open-telemetry/opentelemetry-collector/blob/main/featuregate/README.md) documentation.

cmd/mdatagen/internal/testdata/documentation.md

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,10 @@
22

33
# sample
44

5-
## Feature Gates
5+
## Resource Attributes
66

7-
This component has the following feature gates:
8-
9-
| Feature Gate | Stage | Description | From Version | To Version | Reference |
10-
| ------------ | ----- | ----------- | ------------ | ---------- | --------- |
11-
| `sample.feature.gate` | alpha | This is a sample feature gate for testing purposes | v0.100.0 | N/A | [Link](https://github.com/open-telemetry/opentelemetry-collector/issues/12345) |
12-
| `stable.feature.gate` | stable | This is a stable feature gate | v0.90.0 | v0.95.0 | [Link](https://github.com/open-telemetry/opentelemetry-collector/issues/11111) |
13-
14-
For more information about feature gates, see the [Feature Gates](https://github.com/open-telemetry/opentelemetry-collector/blob/main/featuregate/README.md) documentation.
7+
| Name | Description | Values | Enabled |
8+
| ---- | ----------- | ------ | ------- |
9+
| host.id | The unique host identifier | Any Str | true |
10+
| host.name | The hostname | Any Str | true |
11+
| process.pid | The process identifier | Any Int | true |

cmd/mdatagen/internal/testdata/feature_gates.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ status:
55
beta: [metrics]
66

77
feature_gates:
8-
sample.feature.gate:
8+
- id: sample.feature.gate
99
description: 'This is a sample feature gate for testing purposes'
1010
stage: alpha
1111
from_version: 'v0.100.0'
1212
reference_url: 'https://github.com/open-telemetry/opentelemetry-collector/issues/12345'
1313

14-
stable.feature.gate:
14+
- id: stable.feature.gate
1515
description: 'This is a stable feature gate'
1616
stage: stable
1717
from_version: 'v0.90.0'

cmd/mdatagen/mdatagen

-12.9 MB
Binary file not shown.

cmd/mdatagen/metadata-schema.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,10 @@ telemetry:
260260
# Note: Only the following attribute types are supported: <string|int|double|bool>
261261
attributes: [string]
262262

263-
# Optional: map of feature gate definitions with the key being the gate ID.
263+
# Optional: list of feature gate definitions.
264264
feature_gates:
265-
<gate.id>:
265+
- # Required: unique identifier for the feature gate.
266+
id: <gate.id>
266267
# Required: description of the feature gate.
267268
description: string
268269
# Required: lifecycle stage of the feature gate.

0 commit comments

Comments
 (0)