Skip to content

[mdatagen] Fix panic in TestValue() and TestValueTwo() for single-element or empty enum attributes#15646

Open
bhuvan-somisetty wants to merge 3 commits into
open-telemetry:mainfrom
bhuvan-somisetty:fix-mdatagen-enum-panic
Open

[mdatagen] Fix panic in TestValue() and TestValueTwo() for single-element or empty enum attributes#15646
bhuvan-somisetty wants to merge 3 commits into
open-telemetry:mainfrom
bhuvan-somisetty:fix-mdatagen-enum-panic

Conversation

@bhuvan-somisetty

@bhuvan-somisetty bhuvan-somisetty commented Jul 24, 2026

Copy link
Copy Markdown

Description

Fix index-out-of-range panic in cmd/mdatagen when handling single-element enum attributes or empty enum slices. Also add validation in validateAttributes() and validateResourceAttributes() to reject empty enum attributes.

Link to tracking issue

Fixes #15644

Testing

Added unit tests in cmd/mdatagen/internal/metadata_test.go covering single-element, multi-element, empty enum, and non-enum attributes, as well as metadata validation tests. Verified all tests pass in cmd/mdatagen.

Documentation

None

Authorship

  • I, a human, wrote this pull request description myself.

…ment or empty enum attributes

Fix index-out-of-range panic in Attribute.TestValue() and Attribute.TestValueTwo() when handling attributes with single-element or empty enum slices. Also add validation in validateAttributes() and validateResourceAttributes() for empty Enum slices.
@bhuvan-somisetty

Copy link
Copy Markdown
Author

Hi @dmitryax , @mx-psi whenever you have a moment, could you please approve the CI workflow run for this PR? Thank you!

@osullivandonal

Copy link
Copy Markdown
Contributor

This is very much an edge case, I would argue an enum with less than 2 enumerations is not an enum. Given that if an enum is presented with less than 2 I would expect it to error, something like:

  if len(attr.Enum) == 1 {
      errs = errors.Join(errs, fmt.Errorf("enum for attribute %v must have at least two values", attrName))
  }

If you want to use an enum with one value I would agrue that it should be a constant.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.04%. Comparing base (c823638) to head (2d31d94).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
cmd/mdatagen/internal/metadata.go 57.14% 2 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (57.14%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15646      +/-   ##
==========================================
- Coverage   91.04%   91.04%   -0.01%     
==========================================
  Files         728      729       +1     
  Lines       48467    48497      +30     
==========================================
+ Hits        44129    44154      +25     
- Misses       3011     3018       +7     
+ Partials     1327     1325       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Update validateAttributes and validateResourceAttributes to return a validation error if an enum attribute has fewer than two values. Add unit tests for single-element and empty enum validation.
@bhuvan-somisetty

Copy link
Copy Markdown
Author

Thanks for the feedback @osullivandonal! That makes complete sense — an enum with fewer than 2 values should be rejected as invalid metadata.

I've updated validateAttributes() and validateResourceAttributes() to enforce that enum attributes must have at least two values, and added corresponding unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mdatagen] Fix panic in Attribute.TestValueTwo() and TestValue() when handling single-element or empty enum attributes

2 participants