Skip to content

log connectionId#814

Closed
Dmole wants to merge 1 commit intoapache:mainfrom
Dmole:main_connectionId
Closed

log connectionId#814
Dmole wants to merge 1 commit intoapache:mainfrom
Dmole:main_connectionId

Conversation

@Dmole
Copy link
Copy Markdown
Contributor

@Dmole Dmole commented Jan 30, 2025

Add connectionId to the access log options so the connectionId in the error log can be cross referenced to an IP for debug, fail2ban, etc.

@Dmole Dmole mentioned this pull request Jan 30, 2025
@markt-asf
Copy link
Copy Markdown
Contributor

This is not the way to do this. Look at the ExtendedAccessLogValve. Look at x-H(...) options.

@Dmole Dmole force-pushed the main_connectionId branch from 697ec09 to b4eadfb Compare February 3, 2025 20:07
@Dmole
Copy link
Copy Markdown
Contributor Author

Dmole commented Feb 3, 2025

I added it to both AccessLogValve and ExtendedAccessLogValve because it is non-trivial for users to switch between them as the documentation does not provide a mapping (EG: %v = ?), and the extended valve does not permit arbitrary formatting (EG: JSON log).

is that OK?

@markt-asf
Copy link
Copy Markdown
Contributor

That looks good for for the ExtendedAccessLogValve.

I don't like that this is automatically added as a request attribute. Firstly, if we add this field, why not all the others? Secondly, it seems wrong to add this as an attribute when there is a perfectly good API to obtain the ID. I think that means AccessLogValve needs something like x-H(XXX). The question is what to use since we try and keep this aligned with httpd and we don't know what they'll choose to do in the future. Maybe %{XXX}x?

@rainerjung
Copy link
Copy Markdown
Contributor

rainerjung commented Feb 17, 2025

Concerning analogies in Apache httpd land:

ErrorLogFormat has

%{c}L 	Log ID of the connection
%{C}L 	Log ID of the connection if used in connection scope, empty otherwise

and explains them as

The log ID format %L produces a unique id for a connection or request. This can be used to correlate which log lines belong to the same connection or request, which request happens on which connection. A %L format string is also available in [mod_log_config](https://httpd.apache.org/docs/2.4/en/mod/mod_log_config.html) to allow to correlate access log entries with error log lines. If [mod_unique_id](https://httpd.apache.org/docs/2.4/en/mod/mod_unique_id.html) is loaded, its unique id will be used as log ID for requests.

See: https://httpd.apache.org/docs/2.4/en/mod/core.html#errorlogformat

And as mentioned here, mod_log_config documents:
%L The request log ID from the error log (or '-' if nothing has been logged to the error log for this request). Look for the matching error log line to see what request caused what error.

I haven't checked what the connection id in Tomcat land is and how well this analogy applies.

Furthermore mod_https provides some IDs via httpd env vars (%{...}e):

Variable Name: 	Value Type: 	Description:
HTTP2	flag	HTTP/2 is being used.
H2PUSH	flag	HTTP/2 Server Push is enabled for this connection and also supported by the client.
H2_PUSH	flag	alternate name for H2PUSH
H2_PUSHED	string	empty or PUSHED for a request being pushed by the server.
H2_PUSHED_ON	number	HTTP/2 stream number that triggered the push of this request.
H2_STREAM_ID	number	HTTP/2 stream number of this request.
H2_STREAM_TAG	string	HTTP/2 process unique stream identifier, consisting of connection id and stream id separated by 

@Dmole Dmole force-pushed the main_connectionId branch from b4eadfb to 563245b Compare February 17, 2025 18:54
@Dmole
Copy link
Copy Markdown
Contributor Author

Dmole commented Feb 17, 2025

Added %{c}L and x-H(connectionId) to AccessLogValve and ExtendedAccessLogValve to cross reference errors from catalina.log

@markt-asf
Copy link
Copy Markdown
Contributor

Manually applied as there were a few things I needed to add/change

  • %{xxx}L is for ID attributes not connection attributes
  • new settings needed to be added to the docs
  • change log entry was required
  • The CachedElement interface does not apply here

@markt-asf markt-asf closed this Feb 24, 2025
@Dmole
Copy link
Copy Markdown
Contributor Author

Dmole commented Feb 24, 2025

3cfc9bc

987f737

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