Remove global logger usage

[khushijain21]

What does this PR do?

This PR introduces new methods that accepts a logger. This is done to get rid of global loggers wherever they are used
This is required for beatreceivers to work.

Why is it important?

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.md

Author's Checklist

• [ ]

Related issues

• Relates https://github.com/elastic/ingest-dev/issues/5251

[kruskall]

this is a binary incompatible change

IMO we should pause until the library is in a good enough state to be bumped in all branches then gradually deprecated and remove

[khushijain21]

elastic/beats#46054. This PR will bump the version and accomodate this changeset

dismiss to unblock PR

[kruskall]

This is a breaking change so I'd rather not approve but left a review

[AndersonQ]

This is a breaking change so I'd rather not approve but left a review

This is a breaking change so I'd rather not approve but left a review

It's a good point. Even though we don't do any compatibility promises, we tend to avoid breaking changes.
On the other hand, looking at pkg.go.dev (here and here), it seems only beats/agent or forks of it use the lib.

A simple approach is to deprecate the exiting functions and create new ones receiving a logger. It's a bit more work, but it accomplishes the goal without a breaking change.
For constructor like functions using functional options can be a way to introduce the logger with a backward compatible signature change.

Also, with that, it'd not be necessary to use the global loggers in the functions not used by the beats. They could still use the deprecated version of the functions.

[khushijain21]

It's a good point. Even though we don't do any compatibility promises, we tend to avoid breaking changes.

We've been doing breaking changes to get rid of global logger usages. We are making sure we do them with little impact as possible. There is also a PR up on beats to accomodate this when it is merged elastic/beats#46054

Some references where breaking changes are introduced:
#254
Similarly in elastic-agent-libs elastic/elastic-agent-libs#341

[kruskall]

IMO adding a (deprecated) numcpu func with no arg that defaults to calling the new func with nooplogger should be good enough

[khushijain21]

IMO adding a (deprecated) numcpu func with no arg that defaults to calling the new func with nooplogger should be good enough

Well there is cpu.Load() as well that accepts a loggerl. The new method would then be named as cpu.LoadWithLogger() - mhm doesn't seem quite right

If there is a consensus that we want to do it this way then I'll change it.

[kruskall]

yeah the situation is not great :(
numcpu should not really accept a logger but let the consumer decide how to fallback if there's an error

[khushijain21]

@kruskall , @AndersonQ , can you take a look now? I introduced a new function for numcpu

[VihasMakwana]

Some comments

[VihasMakwana]

Approving as there are no breaking changes anymore. Khushi has introduced new functions and deprecated old ones.