-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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(operations): Create feature flags for all components #1924
Conversation
Signed-off-by: Alexander Rodin <[email protected]>
Signed-off-by: Alexander Rodin <[email protected]>
8759c8a
to
e696b98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Looks like this can make a real difference.
My main reservation is that it adds a bit of an annoying process anytime anyone adds a new component. It's definitely not the end of the world, but I can definitely see myself forgetting to do it. Is this something we're going to try to enforce, or are we ok with it falling behind slightly sometimes?
Signed-off-by: Alexander Rodin <[email protected]>
Signed-off-by: Alexander Rodin <[email protected]>
I've added a CI check which ensures that Vector built without default features has no available components. In case if we find out that it is too restricting, it can be removed later. |
Signed-off-by: Alexander Rodin <[email protected]>
Signed-off-by: Alexander Rodin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! I don't feel super strongly either way, but this seems like a good improvement to developer experience for those who use it.
I wonder if this is the right way to go. It is certainly the least resistance way of improving the rebuild cycle, but complicates the developer experience by requiring additional pieces of configuration that have to be added and removed between builds (albeit on the command line). Is there any way to break vector up into submodules (ex core, sinks, sources, transforms, bin) to reduce the rebuild time of the individual bits? Most of the testing is in the libs, so incremental builds and tests could skip the final main link step which sounds like a big win. |
Signed-off-by: Alexander Rodin <[email protected]>
@bruceg I was thinking about putting each component into a separate library crate too, although I'm not sure how much effort would it take currently. But if we put each component into a separate library crate, I think we would still want to have ability to include/exclude these library crates from the resulting Vector binary. So these feature flags would still be in use. |
Signed-off-by: Alexander Rodin <[email protected]>
Extracting components into subcrates should yield even better results. When crate dependencies don’t change it’s not rebuilt, and this will make most of the work that doesn’t include global changes a lot quicker, while when we do actually work on global stuff the build system will make sure everything is rebuilt properly. |
Related to #1514 |
I think it is even possible to say that this PR closes #1514 🙂
I agree. But it would be more work and requires some planning beforehand, while this PR solves the immediate problem. |
I'm trying to imagine how I would harvest the benefits from feature flags to improve iteration times. I'll have to disable the features that I don't care about at a particular point in time manually, right? I think this is great, definitely an improvement! |
I do it like this:
As a side note: |
I see. In my case, I'll have to also make the |
Signed-off-by: Alexander Rodin <[email protected]>
Signed-off-by: Alexander Rodin <[email protected]>
c67b407
to
0fa5054
Compare
@a-rodin should we document this in the build/compile docs? I could see users wanting this for custom builds, especially embedded devices. |
This may be terminal dependent. On both gnome-terminal and xfce4-terminal on my system, |
We should be careful with these kinds of assumptions. While I agree in theory, I know there have been experience reports from other projects where aggressively splitting out crates has not ended up improving compile times to the degree they expected. |
This is correct, we don't have enough code in our main crate to warrant an extraction imo. Linkerd did this and found nothing, now they have like 30 crates that are much harder to manager than a single crate. |
Signed-off-by: Alexander Rodin <[email protected]>
I agree. The difference I observed was in |
Motivation
As the number of Vector's components steadily grows, the time
cargo check
takes to run grows too.According to my measurements, "warm" runs of
cargo check
currently take at least 8 seconds and cannot be sped up by utilizing multiple CPU cores. This greatly impedes developers' productivity, as the waiting time for getting feedback from the compiler is long enough to make the whole experience non-interactive. For tests the feedback loop becomes even longer, so thatcargo test --lib lua
currently takes at least 21 seconds to run.In order to alleviate this, I propose to put all components behind the corresponding feature flags in
Cargo.toml
to make it possible to build only Vector's core and a given component during development of this component. By default, as one might expect, all components are enabled, but it is possible to runcargo check/test/run
with--no-default-features --features <component>
to compile only the selected components.With this approach
cargo check --no-default-features --features transforms-lua
takes 3 instead of 8 seconds andcargo test --lib --no-default-features --features --transforms-lua lua
takes 5 instead of 21 seconds.Implementation details
{sources,transforms,sinks}-*
were added toCargo.toml
with the corresponding meta-features depending on all of these features.Further thoughts
I've noticed that some components depend on others, in particular
sources-kubernetes
depends onsources-file
,transforms-json_parser
,transforms-regex_parser
;sources-syslog
depends onsources-socket
;sources-vector
depends onsources-socket
;sinks-humio_logs
depends onsinks-splunk_hec
;sinks-new_relic_logs
dependes onsinks-http
;sinks-sematext_logs
depends onsinks-elasticsearc
.I think ideally we might want to factor out the common parts of code into separate crates in
lib
in order to avoid make components orthogonal to each other. I want to open an issue about it tomorrow.In addition, I think some users might like the ability to build slim versions of Vector customized to their needs. It might be especially easy to do if rust-lang/cargo#3126 becomes implemented.
Closes #1514.