Comments (8)
@rohilsurana i see, sounds good. should we do that approach for now (for logrus only) ?
from salt.
what if we also add a similar getter method in the Logger
interface that returns interface{}
. The user can cast it to the specific logger instance that they're using either it is logrus, zap, or anything else.
from salt.
@rahmatrhd I was also thinking that approach as the alternative but returning interface in interface does not looks solid to me personally. But if everybody is okay with that, it is fine.
from salt.
I think its better to not modify the interface because at least the usecase mentioned by @mabdh doesn't really need it, we can get away by providing a getter in Logrus
struct. One more thing I noticed in grpc_middleware
is grpc_logrus
can take *logrus.Entry
so should we instead create a NewEntry
method in Logrus
to avoid users modifying core struct?
from salt.
@kushsharma yes, returning *logrus.Entry
would be more appropriate IMO.
from salt.
For this specific case *logrus.Entry
might suffice.
But for a larger discussion I think we should instead go for more of an injecting approach, where we can inject our own logrus.Logger
instance if needed to initialise. We could use an option for doing this, like -
func LogrusWithLogger(logger *logrus.Logger) Option
This allows us to apply our defaults/options onto the logger as well as keep an handle of it to use it elsewhere or even modify its properties.
We could follow same pattern for other loggers for a similar looking API for all loggers.
from salt.
@rohilsurana even if we do so, how does it solve the problem
we want to get `*logrus.Logger` from salt `log.Logger`
?
Because the log.NewLogrus
will only return the wrapper of *logrus.Logger
which is *log.Logrus
from salt.
We keep a pointer to the injected logrus.Logger
on our side and use it.
from salt.
Related Issues (15)
- Add version package HOT 2
- (config): default path set to `./` not be suitable for all usecases HOT 6
- Add dynamic command grouping support in help HOT 1
- Add ability to export env variables through salt
- feat: support zerolog logger for salt/log
- not getting ConfigFileNotFoundError when config file is not present HOT 1
- add support for grpc loggerV2
- [feat] Custom Error Response with two different Error() functions
- move audit logging pkg from guardian to salt
- Add sql database common package
- Fix zap contextual logging naming
- unified logging pattern & facilities HOT 1
- [bug] cmdx: config read, write and load behave differently
- [salt/printer] Spinner does not allow customising writer
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from salt.