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.

Sign up for GitHub

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(log_to_metric transform): add missing Cargo feature #18308

Merged
merged 1 commit into from
Aug 18, 2023
Merged

chore(log_to_metric transform): add missing Cargo feature #18308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ transforms-logs = [
"transforms-aws_ec2_metadata",
"transforms-dedupe",
"transforms-filter",
"transforms-log_to_metric",
"transforms-lua",
"transforms-metric_to_log",
"transforms-pipelines",
Expand All @@ -591,6 +592,7 @@ transforms-logs = [
transforms-metrics = [
"transforms-aggregate",
"transforms-filter",
"transforms-log_to_metric",
"transforms-lua",
"transforms-metric_to_log",
"transforms-pipelines",
Expand All @@ -603,6 +605,7 @@ transforms-aggregate = []
transforms-aws_ec2_metadata = ["dep:arc-swap"]
transforms-dedupe = ["dep:lru"]
transforms-filter = []
transforms-log_to_metric = []
transforms-lua = ["dep:mlua", "vector-core/lua"]
transforms-metric_to_log = []
transforms-pipelines = ["transforms-filter", "transforms-route"]
Expand Down
4 changes: 4 additions & 0 deletions src/internal_events/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ mod journald;
mod kafka;
#[cfg(feature = "sources-kubernetes_logs")]
mod kubernetes_logs;
#[cfg(feature = "transforms-log_to_metric")]
mod log_to_metric;
mod logplex;
#[cfg(feature = "sinks-loki")]
Expand All @@ -89,6 +90,7 @@ mod mongodb_metrics;
#[cfg(feature = "sources-nginx_metrics")]
mod nginx_metrics;
mod open;
#[cfg(feature = "transforms-log_to_metric")]
Copy link
Member

Choose a reason for hiding this comment

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

This module is actually used in a few places other than log_to_metric so I removed this flagging in #18322

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @jszwedko thanks for the heads-up.
That one was tricky indeed. Can you tell me which component required that parser code?
I did my best to try to find it and couldn't. Also, how come the CI didn't fail for this PR if that was the case?

I had to add that flag to the parser because when compiling Vector with minimal features, I started to get parser import unused errors. I think the correct thing to do is to restore that flag and add the other features that also use it.

Copy link
Member

Choose a reason for hiding this comment

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

Aha, yeah, I see, we will get dead code warnings. I think it may be safer to just revert this temporarily then and let you reintroduce it with the additional flag handling. Let me do that. You can see the flags that failed to compile in isolation here: https://github.com/vectordotdev/vector/actions/runs/5908210923/job/16027341665 (e.g. sources-kubernetes_logs). As you noted, you can see these locally via make check-component-features. Just a warning that this check takes a long time 😓 since it tries to compile each feature individually. We can also trigger CI to run it on a new PR from you if that helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads-up that it's an expensive operation.
I will check out the job output to determine the dependencies and prepare a new PR shortly.
I think it would be a good idea to trigger a CI run for that PR to verify. The second time is the charm!

mod parser;
#[cfg(feature = "sources-postgresql_metrics")]
mod postgresql_metrics;
Expand Down Expand Up @@ -213,6 +215,7 @@ pub(crate) use self::journald::*;
pub(crate) use self::kafka::*;
#[cfg(feature = "sources-kubernetes_logs")]
pub(crate) use self::kubernetes_logs::*;
#[cfg(feature = "transforms-log_to_metric")]
pub(crate) use self::log_to_metric::*;
#[cfg(feature = "sources-heroku_logs")]
pub(crate) use self::logplex::*;
Expand All @@ -224,6 +227,7 @@ pub(crate) use self::lua::*;
pub(crate) use self::metric_to_log::*;
#[cfg(feature = "sources-nginx_metrics")]
pub(crate) use self::nginx_metrics::*;
#[cfg(feature = "transforms-log_to_metric")]
pub(crate) use self::parser::*;
#[cfg(feature = "sources-postgresql_metrics")]
pub(crate) use self::postgresql_metrics::*;
Expand Down
1 change: 1 addition & 0 deletions src/transforms/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub mod aws_ec2_metadata;
pub mod dedupe;
#[cfg(feature = "transforms-filter")]
pub mod filter;
#[cfg(feature = "transforms-log_to_metric")]
pub mod log_to_metric;
#[cfg(feature = "transforms-lua")]
pub mod lua;
Expand Down