Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Logging: Clean up logger building #32174

Closed
nik9000 opened this issue Jul 18, 2018 · 7 comments
Closed

Logging: Clean up logger building #32174

nik9000 opened this issue Jul 18, 2018 · 7 comments
Labels
:Core/Infra/Logging Log management and logging utilities good first issue low hanging fruit help wanted adoptme Meta

Comments

@nik9000
Copy link
Member

nik9000 commented Jul 18, 2018

We build loggers in about 15 different ways right now. After #31588 we'll only really need about 3 different ways. We should remove all of the confusing "variety".
@nik9000 nik9000 added >non-issue :Core/Infra/Logging Log management and logging utilities labels Jul 18, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@nik9000 nik9000 added Meta and removed >non-issue labels Oct 16, 2018
@nik9000 nik9000 mentioned this issue Oct 16, 2018
Merged
@nik9000
Copy link
Member Author

nik9000 commented Oct 16, 2018 •
edited
Loading

I've been working on this one on and off for a while. Right now we have this remaining:

Removing some of the calls to Loggers.getLogger feels like it'd be a pretty good first contribution so I'm going to mark this issue that way.

@nik9000 nik9000 added good first issue low hanging fruit help wanted adoptme labels Oct 16, 2018
@lipsill
Copy link
Contributor

lipsill commented Oct 16, 2018

Hey @nik9000, what do you mean by:

Replace calls to Loggers.getLogger(Class) with LogManager.getLogger(Class)

Remove LogManager.getLogger(Class)

@nik9000
Copy link
Member Author

nik9000 commented Oct 16, 2018

We have this Loggers class that had a lot use historically but for the most part it is just a pass through to Log4j2's LogManager. I'm trying to remove all of the methods from it that I deprecated a few weeks ago.

This is one of those "lets not have 12 ways of doing things just because we accumulated them over the years" initiatives. For the most part the change it super easy and mechanical, it is just that someone has to do them. I've been doing a bunch of them over the past few months but some helpful folks told me last week that I should document my plan for the end state. This is that.

@lipsill
Copy link
Contributor

lipsill commented Oct 17, 2018

Got it! Thanks! So basically step 3. is to finally remove Loggers.getLogger(Class)
I will take over server + server-tests

@nik9000
Copy link
Member Author

nik9000 commented Oct 17, 2018

Got it! Thanks! So basically step 3. is to finally remove Loggers.getLogger(Class)

Right!

I will take over server + server-tests

Awesome!

This was referenced Oct 18, 2018
Merged
Merged
@pratiksanglikar pratiksanglikar mentioned this issue Oct 23, 2018
Merged
nik9000 pushed a commit that referenced this issue Oct 29, 2018
@pratiksanglikar @nik9000
Core: Replace deprecated Loggers calls with LogManager. (#34691)
f1135ef 
Replace deprecated Loggers calls with LogManager.

Relates to #32174
kcm pushed a commit that referenced this issue Oct 30, 2018
@pratiksanglikar @kcm
Core: Replace deprecated Loggers calls with LogManager. (#34691)
8dcf823 
Replace deprecated Loggers calls with LogManager.

Relates to #32174
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Oct 30, 2018
@nik9000
Loggers: Drop last deprecated logger function
e9529ee 
Drop the last function from `Loggers` that just wraps Log4j2.

Relates to elastic#32174
@nik9000 nik9000 mentioned this issue Oct 30, 2018
Merged
@nik9000
Copy link
Member Author

nik9000 commented Oct 30, 2018

Thanks @pratiksanglikar and @lipsill for doing the heavy lifting here!

nik9000 pushed a commit that referenced this issue Oct 30, 2018
@pratiksanglikar @nik9000
Core: Replace deprecated Loggers calls with LogManager. (#34691)
0313211 
Replace deprecated Loggers calls with LogManager.

Relates to #32174
nik9000 added a commit that referenced this issue Oct 31, 2018
@nik9000
Loggers: Drop last deprecated logger function (#35082)
ca620ff 
Drop the last function from `Loggers` that just wraps Log4j2.

Relates to #32174
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Oct 31, 2018
@nik9000
Logger: Merge ESLoggerFactory into Loggers
e3763b4 
`ESLoggerFactory` is now not particularly interesting and simple enought
to fold entirely into `Loggers. So let's do that.

Closes elastic#32174
@nik9000 nik9000 mentioned this issue Oct 31, 2018
Merged
nik9000 added a commit that referenced this issue Nov 1, 2018
@nik9000
Loggers: Drop last deprecated logger function (#35082)
fc918aa 
Drop the last function from `Loggers` that just wraps Log4j2.

Relates to #32174
nik9000 added a commit that referenced this issue Nov 6, 2018
@nik9000
Logger: Merge ESLoggerFactory into Loggers (#35146)
348c28d 
`ESLoggerFactory` is now not particularly interesting and simple enought
to fold entirely into `Loggers. So let's do that.

Closes #32174
nik9000 added a commit that referenced this issue Nov 8, 2018
@nik9000
Logger: Merge ESLoggerFactory into Loggers (#35146)
ffcd5fb 
`ESLoggerFactory` is now not particularly interesting and simple enought
to fold entirely into `Loggers. So let's do that.

Closes #32174
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
:Core/Infra/Logging Log management and logging utilities good first issue low hanging fruit help wanted adoptme Meta
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nik9000 @elasticmachine @lipsill