-
Notifications
You must be signed in to change notification settings - Fork 24
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
Allow to start FLP directly from the flow logs producer #538
Conversation
- New "InProcess" ingest stage - New entry point to start the whole FLP in-process: pipeline.StartFLPInProcess - Add some tests
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #538 +/- ##
==========================================
+ Coverage 66.84% 67.42% +0.57%
==========================================
Files 95 97 +2
Lines 6802 6857 +55
==========================================
+ Hits 4547 4623 +76
+ Misses 1988 1959 -29
- Partials 267 275 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cmd/flowlogs-pipeline/main.go
Outdated
prometheus.SetGlobalMetricsSettings(&cfg.MetricsSettings) | ||
promServer := prometheus.StartServerAsync(&cfg.MetricsSettings.PromConnectionInfo, nil) |
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.
What about making this optionnal ?
If we are looking for lightweight solution we may skip prometheus since we have other export capabilities now
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.
It's not only for prometheus pipeline output but also for the "operational metrics" ie. related to FLP health .. which doesn't mean it cannot be made optional, but I would say the default should still to have these metrics
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.
I can add a field enableOperational
in MetricsSettings.
If it's set false, this "global" server would not be started, and it's still possible to start per-promencode stage servers.
However if it's disabled and the user didn't configure per-promencode servers (which is optional), then they wouldn't get any metric ... so it creates more configuration skews....
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.
maybe enableGlobalServer
would be a better name for the knob as it carries more explicitly that's it's neededif you don't configure per-stage server.
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.
@jpinsonneau done via dbe2422
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.
thanks !
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=8fad274 make set-flp-image |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
The idea behind is to reduce some of the overhead when the agent producing flows, and the FLP workload are running on the same host: in that case, the network connection between them could be avoided, saving resources.
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.