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

Updated the pipes library #736

Merged
merged 5 commits into from
Apr 17, 2024
Merged

Updated the pipes library #736

merged 5 commits into from
Apr 17, 2024

Conversation

mariomac
Copy link
Contributor

@mariomac mariomac commented Apr 12, 2024

Replaces the pipes library by its totally revamped version.

My main concern with the pipes library is that it is full of "magic" and for someone not knowing it internally (someone that it's not me 😅) it's very difficult to reason about how it works. It also had:

  • Provided multiple ways to do the same task, which might be confusing.
  • The annotation-based connection of nodes was not typesafe, and a source of runtime errors with error messages that might not be clarifying.
  • All the magic was based on an intensive use of the reflection API, and the library reached a point of complexity where it was pretty difficult to extend and maintain even for the creator of the library. This was a long-term risk for the team.

I've been lately working on a complete rewrite of the library, which:

  • Provides a unified API for creating graphs.
  • Every configuration is done explicitly via code in a typesafe manner. No more annotations. No more altering the behavior of the nodes via implicit interfaces.
  • The actual code API will be descriptive enough to be able to understand what's happenning in the documentation popups of each function/type, without having to continuously refer to the library tutorials.
  • Reduced from 3500 to 1200 the total lines of source code. Part of the reduction is becase the new version removes a lot of functionalities that remained unused in Beyla.

Replacing the annotations with explicit Go language constructs might add few extra boilerplate code and externally could seem as it does not change much the results, but with this new version, any new contributor has better API guidance to add or change elements in the pipeline.

@mariomac mariomac added do-not-merge WIP or something that musn't be merged wip labels Apr 12, 2024
@grcevski
Copy link
Contributor

I really like the change!!!

@mariomac mariomac added enhancement and removed do-not-merge WIP or something that musn't be merged wip labels Apr 15, 2024
@mariomac mariomac changed the title Draft: test revamping of the pipes library Updated the pipes library Apr 15, 2024
Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM!

@mariomac mariomac merged commit 927dc13 into grafana:main Apr 17, 2024
5 checks passed
@mariomac mariomac deleted the pipes branch April 17, 2024 07:25
mattdurham pushed a commit to mattdurham/beyla that referenced this pull request Jan 22, 2025
* Version ready to share for mod review

* Ported process instrumenter pipeline

* Migrated the whole pipeline

* use v0.10.0 tag of released pipes library
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.

2 participants