Skip to content

chore: sdk/internal/x - generate x package from shared template - #7495

Merged
dmathieu merged 10 commits into
open-telemetry:mainfrom
nikhilmantri0902:imp/sdk_internal_x_gen_x_package
Oct 16, 2025
Merged

chore: sdk/internal/x - generate x package from shared template#7495
dmathieu merged 10 commits into
open-telemetry:mainfrom
nikhilmantri0902:imp/sdk_internal_x_gen_x_package

Conversation

@nikhilmantri0902

Copy link
Copy Markdown
Contributor

Closes #7384

  • Add sdk/internal/gen.go with gotmpl directives to generate x/x.go and x/x_test.go
  • Refactor sdk/internal/x/x.go to generic Feature[T] with multi-key support and Keys()
  • Move Resource feature into sdk/internal/x/features.go (case-insensitive true enables)
  • Add tests: generic feature tests in x_test.go and Resource tests in features_test.go
  • Aligns with sdk/log/internal/x pattern introduced in PR chore: sdk/log/internal/x - generate x package from x component template  #7389

Internal note: replaces Key() with Keys() for internal Feature API. Build and lints pass

- Add sdk/internal/gen.go with gotmpl directives to generate x/x.go and x/x_test.go
- Refactor sdk/internal/x/x.go to generic Feature[T] with multi-key support and Keys()
- Move Resource feature into sdk/internal/x/features.go (case-insensitive true enables)
- Add tests: generic feature tests in x_test.go and Resource tests in features_test.go
- Aligns with sdk/log/internal/x pattern introduced in PR open-telemetry#7389

Internal note: replaces Key() with Keys() for internal Feature API.
Build and lints pass
@codecov

codecov Bot commented Oct 12, 2025

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7495   +/-   ##
=====================================
  Coverage   86.2%   86.2%           
=====================================
  Files        295     294    -1     
  Lines      25864   25844   -20     
=====================================
- Hits       22302   22286   -16     
+ Misses      3189    3185    -4     
  Partials     373     373           
Files with missing lines Coverage Δ
sdk/internal/x/features.go 100.0% <100.0%> (ø)
sdk/internal/x/x.go 100.0% <100.0%> (ø)
sdk/trace/internal/observ/batch_span_processor.go 100.0% <ø> (ø)
sdk/trace/internal/observ/tracer.go 100.0% <ø> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@flc1125 flc1125 added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 13, 2025
@flc1125

flc1125 commented Oct 13, 2025

Copy link
Copy Markdown
Member

Please fix the lint errors; we can verify them with make ci.

@nikhilmantri0902

Copy link
Copy Markdown
Contributor Author

@flc1125 done. Please check.

@MrAlias MrAlias left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should replace sdk/trace/internal/x instead of being added additionally. There should not be more than one x package in a module to prevent duplication and confusion.

The replacement and associated updates should be included here or things should be kept the way they currently are.

@nikhilmantri0902

Copy link
Copy Markdown
Contributor Author

This should replace sdk/trace/internal/x instead of being added additionally. There should not be more than one x package in a module to prevent duplication and confusion.

The replacement and associated updates should be included here or things should be kept the way they currently are.

@flc1125 and @MrAlias should I make changes based on this in this PR or we can take it up later? If for this PR, can you once explain what needs to be done I ll do that

@MrAlias

MrAlias commented Oct 13, 2025

Copy link
Copy Markdown
Contributor

Include the changes in this PR. You "should replace sdk/trace/internal/x instead of [just adding this new package]. There should not be more than one x package in a module to prevent duplication and confusion."

Any dependencies on sdk/trace/internal/x need to be updated to use this added package instead.

…ation

- Delete sdk/trace/internal/x/x.go and sdk/trace/internal/x/features.go
- Experimental flags now live under sdk/internal/x
- Keep README redirect (optional) to guide developers
- Build and lints remain green
…ation

- Delete sdk/trace/internal/x/x.go and sdk/trace/internal/x/features.go
- Experimental flags now live under sdk/internal/x
- Keep README redirect (optional) to guide developers
- Build and lints remain green
@nikhilmantri0902

nikhilmantri0902 commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

@flc1125 @MrAlias I replaced the usages of sdk/trace/internal/x.

Then I thought why would I even need to keep this package. Should I delete this package sdk/trace/internal/x? I tried removing everything, but make ci regenerates the code. Should we just keep those files for now. In the previous commit, they got regenerated when I ran make ci.

@flc1125

flc1125 commented Oct 14, 2025

Copy link
Copy Markdown
Member

Should I delete this package sdk/trace/internal/x?

We can remove them.

Comment thread CHANGELOG.md Outdated
Comment thread sdk/trace/internal/x/README.md Outdated
@dmathieu
dmathieu merged commit f346dec into open-telemetry:main Oct 16, 2025
32 checks passed
@MrAlias MrAlias added this to the v1.39.0 milestone Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

4 participants