add config parameter to suppress go metrics #394
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
KalmanMeth
changed the title
pkg/config/config.go
Outdated
| @@ -38,7 +38,7 @@ type ConfigFileStruct struct { | |||
| LogLevel string `yaml:"log-level,omitempty" json:"log-level,omitempty"` | |||
| Pipeline []Stage `yaml:"pipeline,omitempty" json:"pipeline,omitempty"` | |||
| Parameters []StageParam `yaml:"parameters,omitempty" json:"parameters,omitempty"` | |||
| MetricsSettings MetricsSettings `yaml:"metricsSettings,omitempty" json:"metricsSettings,omitempty"` | |||
| MetricsSettings MetricsSettings `yaml:"metrics-settings,omitempty" json:"metrics-settings,omitempty"` | |||
There was a problem hiding this comment.
Why the camelCase was replaced by kebab-case?
There was a problem hiding this comment.
Changed back. We should consider changing also log-level to be consistent.
There was a problem hiding this comment.
I don't get it ... it was previously kebab-case and not camelCase, no??
On the operator side, it's using metrics-settings to configure FLP (so, changing this would be a breaking change are require operator PR)
There was a problem hiding this comment.
I'm fine either way. Let's just make a decision and try to be consistent.
There was a problem hiding this comment.
IIUC, the conflict rises because MetricsSettings can be set both in the command-line and in the config file.
While the convention of the command-line arguments seems to be kebab-case (log-level is the only other example), the convention of the config file is camelCase.
If we want to keep both conventions and avoid changing the operator's code, I think the easiest solution would be to keep the tags of the fields of ConfigFileStruct that are shared with the command-line arguments formatted as kebab-case.
And the only thing left to do is to revert the change of the command-line argument in
https://github.com/netobserv/flowlogs-pipeline/pull/394/files#diff-704c1b33b10fb781ff935cf83d44f3e6c9a48efdcb46c008463c2223ab43717eR146
@jotak @KalmanMeth WDYT?
Codecov Report
@@ Coverage Diff @@
## main #394 +/- ##
==========================================
- Coverage 61.07% 61.00% -0.07%
==========================================
Files 92 92
Lines 6399 6403 +4
==========================================
- Hits 3908 3906 -2
- Misses 2257 2262 +5
- Partials 234 235 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
KalmanMeth
force-pushed
the
remove-metrics
branch
from
8aa9d9f
to
2f7bdcf
Compare
To suppress the Go metrics, add to the config file: