[component] Make component.Kind a struct #12214
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mx-psi
added
the
release:blocker
mx-psi
force-pushed
the
mx-psi/component-kind-structify
branch
from
72c0238 to
309bea6
Compare
|
cc @bogdandrutu do you think we should do this? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12214 +/- ##
==========================================
- Coverage 91.28% 91.28% -0.01%
==========================================
Files 465 465
Lines 25616 25605 -11
==========================================
- Hits 23384 23373 -11
Misses 1819 1819
Partials 413 413 ☔ View full report in Codecov by Sentry. |
bogdandrutu
approved these changes
Jan 30, 2025
Merged
via the queue into
open-telemetry:main
with commit Jan 31, 2025
a737a48
52 of 53 checks passed
sfc-gh-sili
pushed a commit
to sfc-gh-sili/opentelemetry-collector
that referenced
this pull request
Feb 3, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description <!-- Issue number if applicable --> Changes the underlying type of `component.Kind` to be closer to [`pipeline.Signal`](https://pkg.go.dev/go.opentelemetry.io/collector/pipeline#Signal). Right now the type is defined within `component`, but it could be moved to an internal module so that we could have a different module exposing other value on this enum. This is already doable today, the only thing this PR gives us is 1. slightly more flexibility on things like making the concept of kind more complex (e.g. adding an adjective to a kind). 2. restricting external consumers from implementing their own component kinds without our explicit approval (with some kind of API we design) I am not convinced this is _necessary_ to do, but we may as well do it. This is technically a breaking change since `component.Kind(42)` was a valid expression and it no longer is. I think this is extremely rare, so I suggest if we go ahead we do so in one go. #### Link to tracking issue Fixes open-telemetry#11443
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Changes the underlying type of
component.Kindto be closer topipeline.Signal. Right now the type is defined withincomponent, but it could be moved to an internal module so that we could have a different module exposing other value on this enum.This is already doable today, the only thing this PR gives us is
I am not convinced this is necessary to do, but we may as well do it.
This is technically a breaking change since
component.Kind(42)was a valid expression and it no longer is. I think this is extremely rare, so I suggest if we go ahead we do so in one go.Link to tracking issue
Fixes #11443