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

KER-367: add user info in uwsgi request logs #499

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

danipran
Copy link
Contributor

@danipran danipran commented Jun 3, 2024

No description provided.

@danipran danipran marked this pull request as draft June 4, 2024 05:17
@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from 420fec1 to 596264e Compare June 4, 2024 05:46
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from 596264e to 043c5d3 Compare June 4, 2024 06:01
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from 043c5d3 to cd37436 Compare June 4, 2024 06:35
@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from cd37436 to caaf8dd Compare June 4, 2024 07:07
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from caaf8dd to b97b1c6 Compare June 4, 2024 07:54
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch 2 times, most recently from 6dafa72 to 9f3c8bc Compare June 4, 2024 09:14
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from 9f3c8bc to f7228b2 Compare June 4, 2024 10:13
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from b878be6 to ffe3662 Compare June 4, 2024 13:02
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from ffe3662 to 2541fdb Compare June 5, 2024 07:06
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from 2541fdb to 5087915 Compare June 5, 2024 08:02
@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the KER-367/add-ip-address-in-uwsgi-log branch from 5087915 to 993a330 Compare June 5, 2024 08:16
Copy link

sonarcloud bot commented Jun 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarCloud

@terovirtanen
Copy link
Contributor

KERROKANTASI-API branch is deployed to platta: https://kerrokantasi-pr499.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran marked this pull request as ready for review June 5, 2024 08:30
@danipran danipran requested a review from a team June 5, 2024 08:30
@@ -20,3 +20,8 @@ uwsgi:
ignore-sigpipe: true
ignore-write-errors: true
disable-write-exception: true

logger-req: stdio
log-format: '"remote_addr": "%(addr)", "x_forwarded_for":"%(var.HTTP_X_FORWARDED_FOR)", "request_id":"%(var.HTTP_X_REQUEST_ID)", "remote_user":"%(user)", "bytes_sent":%(size), "request_time":%(secs), "status":%(status), "host":"%(host)", "request_proto":"%(proto)", "path":"%(uri)", "request_length":%(cl), "http_referer":"%(referer)", "http_user_agent":"%(uagent)"'
Copy link
Contributor

@charn charn Jul 31, 2024

Choose a reason for hiding this comment

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

Not sure if a plugin would be required for some json escaping. Some other project have decided to use a plugin. There's some details about the issue here. E.g. atv uses a plugin.

One thing to consider also would be if we would need the log to be in json format or not. 🤔

Seems that there's some issue related to this yaml-configuration. On my machine this produced the following type of log. Note that the ${msg} part in the example below is actually a string which contains data which looks like a json.

{"time":"2024:07:31 13:19:40", "source":"uwsgi-req", '"remote_addr": "172.18.0.1", "x_forwarded_for":"-", "request_id":"-", "remote_user":"-", "bytes_sent":18671, "request_time":0.000206, "status":200, "host":"localhost:8083", "request_proto":"HTTP/1.1", "path":"/static/admin/css/responsive.css", "request_length":0, "http_referer":"http://localhost:8083/admin/login/?next=/admin/", "http_user_agent":"Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0"'}

Tried locally to change the yaml formatted uwsgi configuration into the usual ini format and it was able to produce a more valid json (i.e. the data wasn't contained within a string).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I had a discussion about this with some on in our team... 2 months ago. I recall ending up with removing the JSON escape thing, which I seem to have done here: ffe3662

I'm not quite sure why this would need to be in JSON format in the first place. It seems that other projects do this, but... why? Is it just another KuVa remnant or does it actually have a purpose?

Yeah, I had a lot of trouble with YAML. Most of my force-pushes in the draft stage were because of YAML. I'd prefer INI here, but, well, the configuration's in YAML. I suppose I could rewrite it in INI.

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

Successfully merging this pull request may close these issues.

3 participants