-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[mdatagen] Add optional display_name field to metadata.yaml #14115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[mdatagen] Add optional display_name field to metadata.yaml #14115
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14115 +/- ##
==========================================
- Coverage 92.24% 92.21% -0.04%
==========================================
Files 658 658
Lines 41168 41168
==========================================
- Hits 37977 37963 -14
- Misses 2184 2193 +9
- Partials 1007 1012 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
codeboten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @thc1006, overall it looks good, just one comment
| @@ -1,5 +1,8 @@ | |||
| # Sample Scraper | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would expect the display name to replace the title here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codeboten Thank you for your review and feedback on the display_name implementation.
I've addressed your concern in commit b18acc4. The display_name field now correctly replaces the title in generated
README files rather than creating duplicate headings.
Summary of changes:
- Updated all test component metadata.yaml files with appropriate
display_namevalues - Removed duplicate H1 headings from README files
- Regenerated README files via
make gogenerate
Result:
Each README now has a single H1 heading (from display_name) within the autogenerated section, maintaining clean document structure and matching the expected behavior you described.
All unit tests continue to pass. Please review when you have a chance.
This commit addresses @codeboten's review feedback on PR open-telemetry#14115. The display_name field now properly replaces the title in generated README files instead of creating duplicate headings. Changes: - Update test metadata.yaml files with appropriate display_name values - Clean up README files to remove duplicate titles - Regenerate README files with display_name as the sole H1 heading The README structure is now: - Description text - <!-- status autogenerated section --> - # Component Display Name (from display_name field) - Status table - <!-- end autogenerated section --> Fixes the issue where multiple H1 headings appeared when display_name was set, which was confusing and not the intended behavior. Co-authored-by: SteveYi <[email protected]>
This commit fixes two categories of CI failures:
1. Import Order (goimpi lint failures - 24 errors):
- Fixed mdatagen templates to follow stdThirdPartyLocal scheme
- Separated standard library, third-party, and local imports with blank lines
- Modified templates: package_test.go.tmpl, component_test.go.tmpl,
metrics.go.tmpl, metrics_test.go.tmpl, logs.go.tmpl, logs_test.go.tmpl
- Regenerated all mdatagen generated files with correct import order
- Applied goimports -local go.opentelemetry.io/collector to all generated files
2. Test Expectations (display_name field):
- Updated loader_test.go to expect "Sample Receiver" instead of "Sample"
- This matches the actual display_name value in metadata.yaml
All goimpi lint checks now pass.
All mdatagen tests pass.
Fixes open-telemetry#14115
| This processor is used for testing purposes to check the output of mdatagen. | ||
|
|
||
| <!-- status autogenerated section --> | ||
| # Sample Processor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the title should really be at the top of the document, where it was before. This makes me think we should also have a description field (in this case This processor is used for testing purposes to check the output of mdatagen.) to capture the description of the component. Then the header tag <!-- status autogenerated section --> could be moved to the very top of the document and all the generated contents would be in the right place
- Added display_name field to metadata-schema.yaml - Updated Metadata struct to include DisplayName field - Implemented automatic capitalization of type field when display_name not provided - Modified readme.md.tmpl to generate README titles from display_name - Fixed unused import issue in config_test.go.tmpl - Added test cases for display_name handling - Regenerated internal test code and README files Fixes open-telemetry#14114 Co-authored-by: SteveYi <[email protected]> Signed-off-by: thc1006 <[email protected]>
- Add 'nodisplayname' to spell-check dictionary - Remove incorrect auto-generation of DisplayName in loader.go - Update test expectations for optional display_name field - Add explicit display_name to test metadata files Co-authored-by: SteveYi <[email protected]>
This commit addresses @codeboten's review feedback on PR open-telemetry#14115. The display_name field now properly replaces the title in generated README files instead of creating duplicate headings. Changes: - Update test metadata.yaml files with appropriate display_name values - Clean up README files to remove duplicate titles - Regenerate README files with display_name as the sole H1 heading The README structure is now: - Description text - <!-- status autogenerated section --> - # Component Display Name (from display_name field) - Status table - <!-- end autogenerated section --> Fixes the issue where multiple H1 headings appeared when display_name was set, which was confusing and not the intended behavior. Co-authored-by: SteveYi <[email protected]>
This commit fixes two categories of CI failures:
1. Import Order (goimpi lint failures - 24 errors):
- Fixed mdatagen templates to follow stdThirdPartyLocal scheme
- Separated standard library, third-party, and local imports with blank lines
- Modified templates: package_test.go.tmpl, component_test.go.tmpl,
metrics.go.tmpl, metrics_test.go.tmpl, logs.go.tmpl, logs_test.go.tmpl
- Regenerated all mdatagen generated files with correct import order
- Applied goimports -local go.opentelemetry.io/collector to all generated files
2. Test Expectations (display_name field):
- Updated loader_test.go to expect "Sample Receiver" instead of "Sample"
- This matches the actual display_name value in metadata.yaml
All goimpi lint checks now pass.
All mdatagen tests pass.
Fixes open-telemetry#14115
e5646be to
ce112ad
Compare
Add description field to capture component purpose in generated README. The description is rendered after the title within the autogenerated section. Addresses review feedback from PR open-telemetry#14115 Related to open-telemetry#14114
Update generated files after schema changes to include description field support. The regeneration also includes upstream changes from rebase (semconv v1.37.0, SystemCPUTime metric).
Description
This PR adds an optional
display_namefield to metadata.yaml for human-readable component names, as requested in #14114.Changes
display_namefield to metadata-schema.yamlImplementation
Following the implementation guide from @mx-psi:
Behavior
display_nameis set in metadata.yaml, it will be used as the README titleTesting
Fixes #14114
Co-authored-by: SteveYi [email protected]