Skip to content
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

[chore] Add pipeline module #11209

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Sep 18, 2024

Description

To facilitate the work in #11204 as some breaking changes and some deprecations, this PR adds the new pipeline module separately so that future PRs can handle the breaking changes and deprecations.

In order to make Signal uninstantiable outside of this repo, while still being extendable in places like componentprofiles, a new internal module is added to handle the Signal logic. To reduce the dependency sprawl that would happen if signal was an internal package in go.opentelemetry.io/collector, I made it a module, similar to globalgates.

Link to tracking issue

Related to #10947

Testing

Added unit tests

Documentation

Added godoc comments

Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 93.84615% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.82%. Comparing base (df3c9e3) to head (f75f511).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
internal/globalsignal/signal.go 86.66% 1 Missing and 1 partial ⚠️
pipeline/pipeline.go 96.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11209      +/-   ##
==========================================
- Coverage   91.87%   91.82%   -0.06%     
==========================================
  Files         421      424       +3     
  Lines       19989    20067      +78     
==========================================
+ Hits        18365    18426      +61     
- Misses       1249     1264      +15     
- Partials      375      377       +2     

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

@mx-psi
Copy link
Member

mx-psi commented Sep 18, 2024

We discussed the pipeline package name on the 2024-09-18 Collector stability meeting, unless anybody proposes a different name by EOW I will merge this

cc @open-telemetry/collector-approvers

@TylerHelmuth TylerHelmuth requested a review from a team as a code owner September 19, 2024 17:55
@mx-psi mx-psi merged commit 0c7d347 into open-telemetry:main Sep 20, 2024
46 of 49 checks passed
@github-actions github-actions bot added this to the next release milestone Sep 20, 2024
@TylerHelmuth TylerHelmuth deleted the add-pipeline-module branch September 20, 2024 17:35
codeboten pushed a commit that referenced this pull request Sep 23, 2024
#### Description
Depends on
#11209

This PR is a non-breaking implementation of
#10947. It
adds a new module, `pipeline`, which houses a `pipeline.ID` and
`pipeline.Signal`. `pipeline.ID` is used to identify a pipeline within
the service. `pipeline.Signal` is uses to identify the signal associated
to a pipeline.

I do this work begrudgingly. As the PR shows, this is a huge refactor
when done in a non-breaking way, will require 3 full releases, and
doesn't benefit our [End Users or, in my opinion, our Component
Developers or Collector Library
Users](https://github.com/open-telemetry/opentelemetry-collector/blob/main/CONTRIBUTING.md#target-audiences).
I view this refactor as a Nice-To-Have, not a requirement for Component
1.0.

<!-- Issue number if applicable -->
#### Link to tracking issue
Works towards
#9429
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this pull request Sep 24, 2024
…emetry#11204)

#### Description
Depends on
open-telemetry#11209

This PR is a non-breaking implementation of
open-telemetry#10947. It
adds a new module, `pipeline`, which houses a `pipeline.ID` and
`pipeline.Signal`. `pipeline.ID` is used to identify a pipeline within
the service. `pipeline.Signal` is uses to identify the signal associated
to a pipeline.

I do this work begrudgingly. As the PR shows, this is a huge refactor
when done in a non-breaking way, will require 3 full releases, and
doesn't benefit our [End Users or, in my opinion, our Component
Developers or Collector Library
Users](https://github.com/open-telemetry/opentelemetry-collector/blob/main/CONTRIBUTING.md#target-audiences).
I view this refactor as a Nice-To-Have, not a requirement for Component
1.0.

<!-- Issue number if applicable -->
#### Link to tracking issue
Works towards
open-telemetry#9429
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants