Skip to content

Conversation

@ANAMASGARD
Copy link
Contributor

Description

Add guidelines to coding-guidelines.md explaining why embedded structs should be avoided in configuration definitions. This helps prevent unmarshal issues, naming conflicts, and improves code clarity.

Key points added to the documentation:

  • Unmarshal Compatibility: Embedded structs can break custom Unmarshal implementations
  • Naming Conflicts: Embedded structs can cause field name collisions when multiple embedded types have fields with the same name
  • Clarity: Named fields make the configuration structure more explicit and easier to understand
  • Example code: Shows the difference between BAD (embedded) and GOOD (named fields) patterns

Link to tracking issue

Fixes #12719

Testing

Documentation only - no code changes. No testing required.

Documentation

Added new subsection "Avoid Embedded Structs" under the existing "Configuration structs" section in docs/coding-guidelines.md. The section includes:

  • Clear explanation of what embedded structs are
  • Three key reasons to avoid them in configurations
  • Code example demonstrating the preferred approach using named fields with mapstructure tags

Add guidelines to coding-guidelines.md explaining why embedded structs
should be avoided in configuration definitions. This helps prevent
unmarshal issues, naming conflicts, and improves code clarity.

Fixes open-telemetry#12719
@ANAMASGARD ANAMASGARD requested a review from a team as a code owner December 31, 2025 16:05
@ANAMASGARD ANAMASGARD requested a review from dmathieu December 31, 2025 16:05
@dmathieu dmathieu added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 5, 2026
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 20, 2026
ANAMASGARD and others added 2 commits January 21, 2026 13:02
- Remove changelog entry (not needed for doc-only changes per dmathieu)
- Add documentation about using squash tag with named fields to achieve
  flat YAML structure while avoiding true embedding (per evan-bradley)
@ANAMASGARD
Copy link
Contributor Author

@dmathieu I'm not aware of a linter that specifically checks for embedded structs in config files. This could be a good enhancement for the future as a custom linter rule. For now, the documentation should help maintain awareness among contributors.

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.79%. Comparing base (f724e49) to head (8fadf87).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14340      +/-   ##
==========================================
- Coverage   91.81%   91.79%   -0.03%     
==========================================
  Files         679      679              
  Lines       42857    42857              
==========================================
- Hits        39351    39340      -11     
- Misses       2439     2446       +7     
- Partials     1067     1071       +4     

☔ View full report in Codecov by Sentry.
📢 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.

@ANAMASGARD ANAMASGARD requested review from dmathieu and jmacd January 22, 2026 03:28
@github-actions github-actions bot removed the Stale label Jan 22, 2026
Copy link
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to re-request approvals for every merge with main.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 6, 2026
@evan-bradley evan-bradley removed the Stale label Feb 6, 2026
@evan-bradley evan-bradley added the ready-to-merge Code review completed; ready to merge by maintainers label Feb 10, 2026
@codeboten codeboten enabled auto-merge February 10, 2026 23:30
@codeboten codeboten added this pull request to the merge queue Feb 10, 2026
Merged via the queue into open-telemetry:main with commit 7fd6565 Feb 11, 2026
46 of 48 checks passed
@otelbot
Copy link
Contributor

otelbot bot commented Feb 11, 2026

Thank you for your contribution @ANAMASGARD! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

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

Labels

ready-to-merge Code review completed; ready to merge by maintainers Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document avoiding embedded structs for configs (and in general if possible)

5 participants