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

Introduce typed pipeline #2896

Merged
merged 9 commits into from
Apr 30, 2024
Merged

Conversation

jaronoff97
Copy link
Contributor

@jaronoff97 jaronoff97 commented Apr 24, 2024

Description: Introduces a typed pipeline for the collector. This allows us to really easily construct a list of enabled components.

NOTE! One decision we need to make:

By using a struct like this, we have to figure out how to handle an empty list of processors. I chose to always include it on marshal i.e. if someone has a pipeline configuration like this:


service:
  extensions:
    - oauth2client
  pipelines:
    traces:
      receivers:
        - otlp
      exporters:
        - otlp/auth

we will then unmarshal and remarshal it to be:


service:
  extensions:
    - oauth2client
  pipelines:
    traces:
      receivers:
        - otlp
      processors: []
      exporters:
        - otlp/auth

IMO this is fine and better than the reverse where we set it explicitly to an empty list and then don't remarshal it with that empty list. If someone has a way to make this work both ways, let me know :)

DECISION

From the SIG we decided this is fine, it's only visible to a user in their configmap, but no functionality difference.

Link to tracking Issue(s):

part of #2603

Testing: unit testing

Documentation: n/a

@jaronoff97 jaronoff97 marked this pull request as ready for review April 26, 2024 18:02
@jaronoff97 jaronoff97 requested a review from a team April 26, 2024 18:02
Copy link
Contributor

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

LGTM overall, with one question I'd like to answer before we merge.

@jaronoff97 jaronoff97 requested a review from swiatekm April 30, 2024 17:53
@jaronoff97 jaronoff97 added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 30, 2024
@jaronoff97 jaronoff97 enabled auto-merge (squash) April 30, 2024 18:42
@jaronoff97 jaronoff97 merged commit 2b7d4b3 into open-telemetry:main Apr 30, 2024
31 of 32 checks passed
@jaronoff97 jaronoff97 deleted the cleanup-part-1 branch April 30, 2024 19:04
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* introduce more typing for pipelines

* rock and a hard place

* resolve tests

* Fix tests

* chlog

* fix docs

* thing

* alphabet

* remove chlog entry
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* introduce more typing for pipelines

* rock and a hard place

* resolve tests

* Fix tests

* chlog

* fix docs

* thing

* alphabet

* remove chlog entry
rubenvp8510 pushed a commit to rubenvp8510/opentelemetry-operator that referenced this pull request May 7, 2024
* introduce more typing for pipelines

* rock and a hard place

* resolve tests

* Fix tests

* chlog

* fix docs

* thing

* alphabet

* remove chlog entry
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants