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.

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

Logging and log-rotation. #65

Open
kfogel opened this issue Jun 23, 2016 · 4 comments
Open

Logging and log-rotation. #65

kfogel opened this issue Jun 23, 2016 · 4 comments

Comments

@kfogel
Copy link
Member

kfogel commented Jun 23, 2016

This is a successor to issue #20, and subsumes that issue:

We need to add a log event whenever an endpoint is called, and we need to document how to configure logging and, if necessary, set up log rotatation. The infrastructure for the first part is in place: we're already importing and using log4j. See src/main/resources/log4j.xml. See also commit a131612, and maybe commit 1b3de81, for an example of how to instantiate and invoke the logger.

(By the way, commit e25c7cb has the addition of the log4j.xml file, although that commit was mostly about offering better API responses in error cases.)

kfogel added a commit that referenced this issue Jul 15, 2016
Implement basic logging of a GET request on /clients, including having
the log entry show the number of clients returned in the response.
@kfogel
Copy link
Member Author

kfogel commented Jul 15, 2016

One thing that's not yet clear to me is if we want all our regular logging to happen at the DEBUG level, or if the normal thing to do is have logs not show debugging output (i.e., assuming the app isn't being run in debug mode) but still show intentionally logged regular events such as GET requests on /clients. Need to research standard log4js usage, and general logging conventions, a bit more.

cecilia-donnelly pushed a commit that referenced this issue Jul 22, 2016
Add more logging calls to the clients endpoint.  Next I'll add these to
the rest of the endpoints.

The Log4J config file determines how detailed to get in the
logging. INFO is slightly less detailed than DEBUG, so with this change
we'll get less logging output total (in both catalina.out and
OpenHMIS.log), but we will get all the INFO calls we add to the
endpoints.  Each level is inclusive of the more severe levels, so now
that we've set the logging level to INFO we'll see everything in the
INFO, WARN, ERROR, and FATAL categories, but not the lower level
categories (TRACE and DEBUG).  See
https://logging.apache.org/log4j/2.x/manual/configuration.html for more.

For future debugging we can always change the logging level in log4j.xml
to show more detail (e.g., back to DEBUG).  For now, though, using INFO
should reduce the amount of logging, as requested in #72.
cecilia-donnelly pushed a commit that referenced this issue Jul 25, 2016
We still might want to include more information, but this should let us
know whenever any endpoint is hit.
cecilia-donnelly pushed a commit that referenced this issue Jul 26, 2016
Note that we may add more logging information and/or change the format.
@cecilia-donnelly
Copy link
Contributor

Merged to master in 634fcbe, but as mentioned in that commit we may yet update the format / add more information to the logging.

cecilia-donnelly pushed a commit that referenced this issue Jul 27, 2016
Make new user columns editable in the UserManager, and add them to the
admin page for editing and creating users.

Note that I added some logging to the User endpoint (#65) which should
have been added in 1976919.
@cecilia-donnelly
Copy link
Contributor

cecilia-donnelly commented Jul 29, 2016

I believe, based on @kfogel's review, that we'll use these formats for all logging statements:

  • For a GET of many objects: "GET <endpoint> (<number> results)"
  • For a GET/PUT/DELETE of one object: "<method> <endpoint>/<id>"
  • For a POST that creates an object: "POST <endpoint> (New id: <id>)"

I think Karl wrote that up somewhere, but I couldn't find it. Please link if you see it!

@kfogel
Copy link
Member Author

kfogel commented Jul 29, 2016

Ah, miscommunication, sorry. I thought you were going to write it up, you thought I was going to write it up. I sort of did write it up, in issue #76, but we really should have it in the tree...

Okay, done in commit 20315dd. Some day when we have server administration documentation, separate from INSTALL.md, the new material can be moved, but for now at least it's in-tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants