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

Fix upstream_response_time parsing with colon in value #267

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

kralewitz
Copy link
Contributor

The fix for correct parsing of multiple values in upstream_response_time
applied in #221 was only partial, as it accounts only for commas, but
the value can also contain colons as separators, when the request goes
through an internal redirect.

See the description for '$upstream_addr' in
https://nginx.org/en/docs/http/ngx_http_upstream_module.html for more
info.

The fix for correct parsing of multiple values in upstream_response_time
applied in martin-helmich#221 was only partial, as it accounts only for commas, but
the value can also contain colons as separators, when the request goes
through an internal redirect.

See the description for '$upstream_addr' in
https://nginx.org/en/docs/http/ngx_http_upstream_module.html for more
info.
@codeclimate
Copy link

codeclimate bot commented Jul 12, 2022

Code Climate has analyzed commit 2d793ab and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@helmich-bot
Copy link
Collaborator

There has not been any activity to this pull request in the last 30 days. It will automatically be closed after 7 more days. Remove the stale label to prevent this.

@Nereis
Copy link

Nereis commented Aug 22, 2022

In relatation of #274 , I tried to test it but the issue doesn't look to be fixed (hoping I didn't mess up the test, see below)

git clone https://github.com/kralewitz/prometheus-nginxlog-exporter.git
go build
scp ./prometheus-nginxlog-exporter ...:/usr/bin/prometheus-nginxlog-exporter
systemctl daemon-reload
systemctl start nginx-log-exporter.service
journalctl -u nginx-log-exporter -f
Aug 22 09:06:56 xxx prometheus-nginxlog-exporter[48081]: 
	error while parsing line
		'[22/Aug/2022:09:06:56 +0200] client=192.168.3.26 remote_user="-" request="GET /jira/rest/api/2/myself?expand=groups%2CapplicationRoles HTTP/1.1" request_length=660 status=200 body_bytes_sent=738 referer="https://xxx" user_agent="Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:103.0) Gecko/20100101 Firefox/103.0" x_forwarded="xxx, xxx" request_time=0.016 upstream_response_time=0.004, 0.012 upstream_connect_time=0.004, 0.004 upstream_header_time=0.004, 0.012 subnet=192.168.3.0'
	text log parsing err: access log line 
		'[22/Aug/2022:09:06:56 +0200] client=192.168.3.26 remote_user="-" request="GET /jira/rest/api/2/myself?expand=groups%2CapplicationRoles HTTP/1.1" request_length=660 status=200 body_bytes_sent=738 referer="https:/xxx" user_agent="Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:103.0) Gecko/20100101 Firefox/103.0" x_forwarded="xxx, xxx" request_time=0.016 upstream_response_time=0.004, 0.012 upstream_connect_time=0.004, 0.004 upstream_header_time=0.004, 0.012 subnet=192.168.3.0' 
	does not match given format
		'^\[(?P<time_local>[^]]*)\] client=(?P<remote_addr>[^ ]*) remote_user="(?P<remote_user>[^"]*)" request="(?P<request>[^"]*)" request_length=(?P<request_length>[^ ]*) status=(?P<status>[^ ]*) body_bytes_sent=(?P<body_bytes_sent>[^ ]*) referer="(?P<http_referer>[^"]*)" user_agent="(?P<http_user_agent>[^"]*)" x_forwarded="(?P<http_x_forwarded_for>[^"]*)" request_time=(?P<request_time>[^ ]*) upstream_response_time=(?P<upstream_response_time>[^ ]*) upstream_connect_time=(?P<upstream_connect_time>[^ ]*) upstream_header_time=(?P<upstream_header_time>[^ ]*)'

I don't think you can fix it by splitting the resulted variable. The error indicates that the regex can't parse meaning we are before the variable has been parsed.

I think the issue comes from the translation of the nginx format to regex which is apparently done with gonx

@kralewitz
Copy link
Contributor Author

@Nereis you need to quote the $upstream_response_time variable in the log_format directive in nginx config and in the format field of nginxlog exporter as well, so they match.

@martin-helmich martin-helmich merged commit 81de497 into martin-helmich:master Oct 11, 2022
@martin-helmich martin-helmich linked an issue Oct 11, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple upstream values not supported
4 participants