[Chore] Replace global logger with local loggers in mb modules #5#44107
[Chore] Replace global logger with local loggers in mb modules #5#44107pierrehilbert merged 16 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
Pinging @elastic/integrations (Team:Integrations) |
|
cc: @lalit-satapathy, @consulthys , @gizas |
|
LGTM especially for k8s and docker modules |
|
/test |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
|
||
| groups := makeNameSet(test.groups...).pred() | ||
| topics := makeNameSet(test.topics...).pred() | ||
| err := fetchGroupInfo(collectEvents, test.client, groups, topics) |
There was a problem hiding this comment.
In some places, we have used "", selector, "test" as values in the second param of NewTestingLogger. Should we use "test" instead of "" in all *_test.go files?
There was a problem hiding this comment.
By default, I have used an empty selector. But if there existed a logger setup with "test" as the selector - "test" is used with NewTestingLogger as well. This is to stay consistent with the old code.
| } | ||
|
|
||
| // lookup ips for all brokers | ||
| debugf("match by ips") |
There was a problem hiding this comment.
why not use: m.logger.Named("kafka")
why all debugf calls are not replaces in a similar way?
There was a problem hiding this comment.
This specific one was a typo. Thanks for catching this.
There was a problem hiding this comment.
why all debugf calls are not replaces in a similar way?
All are replaced in a similar way. I did not find such discrepancy anywhere else, have you?
There was a problem hiding this comment.
ah na. Did not open the full file; I just asked if there are more or not. If there's not, LGTM.
| // do not report events since this is not a leader | ||
| if res.StatusCode == http.StatusForbidden && | ||
| retMessage == msgValueNonLeader { | ||
| if m.debugEnabled { |
There was a problem hiding this comment.
Are we sure that something like m.logger.IsDebug() or something like this is not required; it's handled already with Debugf?
There was a problem hiding this comment.
yes, so m.debugEnabled would check if the particular selector can be logged at debug level. i.e if debug level is configured
m.logger.Debugf() will only log message when debug level is configured.
It is achieving the same functionality
…4107) * [Chore] Replace global logger with local loggers in mb modules #5 * fix tests * make check * make check * fix tests * make update * fix redis integ test * deploy on k8's * add deps * fix logger name for kafka (cherry picked from commit 11713f0) # Conflicts: # x-pack/metricbeat/module/awsfargate/task_stats/task_stats.go
(elastic#44107) * [Chore] Replace global logger with local loggers in mb modules elastic#5 * fix tests * make check * make check * fix tests * make update * fix redis integ test * deploy on k8's * add deps * fix logger name for kafka
(elastic#44107) * [Chore] Replace global logger with local loggers in mb modules elastic#5 * fix tests * make check * make check * fix tests * make update * fix redis integ test * deploy on k8's * add deps * fix logger name for kafka
…les #5 (#45883) * [Chore] Replace global logger with local loggers in mb modules #5 (#44107) * [Chore] Replace global logger with local loggers in mb modules #5 * fix tests * make check * make check * fix tests * make update * fix redis integ test * deploy on k8's * add deps * fix logger name for kafka * add config
Proposed commit message
This PR replaces global logger instances with local loggers in metricbeat modules
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Related issues