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

fixes #15863 - add short request UUID to logs #3678

Closed
wants to merge 1 commit into from

Conversation

domcleal
Copy link
Contributor

New middleware stores the request ID (either from X-Request-ID or a
UUID generated by Rails) in the logger's thread storage. A short UUID is
now logged by default to minimise size of the log messages.


Generates log messages such as:

$ egrep "33b38af1.*(Started|Completed)" log/development.log 
2016-07-27T15:38:43 33b38af1 [app] [I] Started GET "/" for 127.0.0.1 at 2016-07-27 15:38:43 +0100
2016-07-27T15:38:45 33b38af1 [app] [I] Completed 200 OK in 1978ms (Views: 1695.6ms | ActiveRecord: 18.5ms)

@ohadlevy
Copy link
Member

ohadlevy commented Jul 28, 2016

perfect timing for my ha setup at https://github.com/ohadlevy/foreman-docker-compose thanks @domcleal - however does it make more sense to use session id vs request id? e.g. if you would like to follow a user requests across servers - afaiu this changes every time you access foreman?

@lzap
Copy link
Member

lzap commented Jul 28, 2016

Well, both are quite valuable and in the future we want to send structured logs into journal. Since we are dropping EL6 soon, this will enable us to send as much info as we want.

New middleware stores the session and request IDs (either from
X-Request-ID or a UUID generated by Rails) in the logger's thread
storage. The truncated session ID is now logged by default to minimise
size of the log messages.
@domcleal
Copy link
Contributor Author

Yes, session ID is preferable, I've updated the PR to use that by default. Both are stored and the request ID is used if the session ID is missing (i.e. first request without cookies).

@lzap
Copy link
Member

lzap commented Jul 28, 2016

An interesting idea: Can we send the UUID (whatever this will be) to Smart Proxy HTTP requests in a header? This way we can make similar change in Smart Proxy and log both Foreman request/session UUID and it's own request UUID.

@lzap
Copy link
Member

lzap commented Jul 28, 2016

We already have our log buffer feature and theoretically we could show logs from particular transaction in the Foreman UI, we are that close from that!

@ohadlevy
Copy link
Member

@lzap it makes sense, you probably want to include also the effective foreman user (e.g. for api requests and because the uuid means nothing besides the foreman logs).
if we had a foreman own session - like https://github.com/blog/1661-modeling-your-app-s-user-session) that would make it even more interesting :)

@ohadlevy
Copy link
Member

merged as d97deb4 thanks @domcleal

@ohadlevy ohadlevy closed this Jul 28, 2016
@lzap
Copy link
Member

lzap commented Jul 28, 2016

Ok, an unit test would be plus, but whatever.

I filed http://projects.theforeman.org/issues/15876 and http://projects.theforeman.org/issues/15877 to track the idea.

@lzap
Copy link
Member

lzap commented Jul 28, 2016

Damn, these are dupes. I'd love to complain on RedMine searching capabilities, but @domcleal is able to find tickets, so it must be me... ;-)

@@ -53,7 +55,7 @@
:log_trace: false
:level: info
:type: file
:pattern: "%d [%c] [%.1l] %m\n"
:pattern: "%d %.8X{session} [%c] [%.1l] %m\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of having the session here vs. request id? The issue here is with user performing multiple requests at the same time. Also using the session id as a shared UI between other services that are called as part of the request is limiting, as you can't group the log messages by user requests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see @ohadlevy had the idea of using session id here: could we revisit this: I find this not that useful as having always request id there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss on #4410

@iNecas
Copy link
Member

iNecas commented Mar 24, 2017

The user is now being logged in as part of #4385

tbrisker added a commit to tbrisker/foreman that referenced this pull request Jan 1, 2020
Also added a few useful pry plugins that help debugging.
tbrisker added a commit to tbrisker/foreman that referenced this pull request Jan 14, 2020
Also added a few useful pry plugins that help debugging.
ezr-ondrej pushed a commit that referenced this pull request Jan 14, 2020
Also added a few useful pry plugins that help debugging.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants