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

fix(metrics): allow each metric output channel to be selected independently #3232

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions userspace/falco/app/actions/process_events.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,9 +413,9 @@ static falco::app::run_result init_stats_writer(
return falco::app::run_result::fatal("Metrics interval was passed as numeric value without Prometheus time unit. Please specify a time unit");
}

if (config->m_metrics_enabled && !sw->has_output())
if (config->m_metrics_enabled && !(sw->has_output() || config->m_webserver_config.m_prometheus_metrics_enabled))
{
return falco::app::run_result::fatal("Metrics are enabled with no output configured. Please enable at least one output channel");
return falco::app::run_result::fatal("Metrics are enabled with no output configured. Please enable at least one output channel ('metrics.output_rule', 'metrics.output_file' or 'webserver.prometheus_metrics_enabled')");
}

falco_logger::log(falco_logger::level::INFO, "Setting metrics interval to " + config->m_metrics_interval_str + ", equivalent to " + std::to_string(config->m_metrics_interval) + " (ms)\n");
Expand Down
4 changes: 2 additions & 2 deletions userspace/falco/falco_metrics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,15 @@ std::string falco_metrics::to_text(const falco::app::state& state)
{
fs::path fs_path = item.first;
std::string metric_name_file_sha256 = fs_path.filename().stem();
metric_name_file_sha256 = "falco.sha256_rules_file." + falco::utils::sanitize_metric_name(metric_name_file_sha256);
metric_name_file_sha256 = "falco_sha256_rules_file_" + falco::utils::sanitize_metric_name(metric_name_file_sha256);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was a super small regression I must have introduced in the last PR. We will push some better checks and sanitizing the metrics name into libs in the upcoming dev cycle, already tracking it in our issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Strange, I thought I saw this dot that look out of place but somehow missed to ask about it...

I think we should add a more thorough test of the output content to avoid such regression in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, plus even push more checks into libs for which we didn't have time anymore. It's tracked on that one issue we have.

prometheus_text += prometheus_metrics_converter.convert_metric_to_text_prometheus(metric_name_file_sha256, "falcosecurity", "falco", {{metric_name_file_sha256, item.second}});
}

for (const auto& item : state.config.get()->m_loaded_configs_filenames_sha256sum)
{
fs::path fs_path = item.first;
std::string metric_name_file_sha256 = fs_path.filename().stem();
metric_name_file_sha256 = "falco.sha256_config_file." + falco::utils::sanitize_metric_name(metric_name_file_sha256);
metric_name_file_sha256 = "falco_sha256_config_file_" + falco::utils::sanitize_metric_name(metric_name_file_sha256);
prometheus_text += prometheus_metrics_converter.convert_metric_to_text_prometheus(metric_name_file_sha256, "falcosecurity", "falco", {{metric_name_file_sha256, item.second}});
}
#endif
Expand Down
Loading