Allow customization for context of LogEntry instances created by LoggingHandler #1329
Comments
This sounds good to me.
I am not sure this is a feature that a lot of users would use/need. |
|
@mziccard Do you want a PR for the change to protected? With regards to a threadlocal mechanism, I agree it is not entirely widely applicable, but neither does it have a zero use-case. Other logging mechanisms (Eg logback MDC) do provide this feature and indeed the stackdrive log API has the map of labels as an extensible mechanism for such logging that is essentially hidden by the JUL handler. If there is zero interest from you guys for this feature, I'll drop it. But if there is a hint of interest I'm happy to create a PR to show how it may be done with minimal cost to users that do not use it. |
@gregw I'd be happy to have such a PR. If we can make its cost close-to-zero it'd be nice to have.
Should we put this on hold while we evaluate the threadlocal mechanism? |
Allow extended classes to access the LogEntry building, so that additional information may be encoded in each log entry (eg traceid). Use addLabel rather than setLabels as it avoids the creation and copy of a HashMap per log entry. Signed-off-by: Greg Wilkins <[email protected]>
|
@mziccard I considered a few ways to do the ThreadLocal, and I'm not sure there is a one size fits all approach. The PR #1385 is a minimal approach to at least allow me to progress on https://github.com/GoogleCloudPlatform/jetty-runtime without needing to copy over the entire LoggingHandler. The down side of using inheritance rather than pluggability is that the one external impl cannot work for both AsyncLoggingHandler and LoggingHandler, but at least an external impl is possible. |
removed extra protected improved extension signature
Removed buildEntryFor
Cleanedup imports
* Issue #1329 Allow extended classes to access the LogEntry building, so that additional information may be encoded in each log entry (eg traceid). Use addLabel rather than setLabels as it avoids the creation and copy of a HashMap per log entry. Signed-off-by: Greg Wilkins <[email protected]>
|
Resolved in #1385 |
|
Is there an ETA on a 6.1 release with this change? |
|
I'm aiming to do a release by EOD tomorrow - I have a couple more things to include. |
…ator_java versions (#1329) - [ ] Regenerate this pull request now. PiperOrigin-RevId: 472750037 Source-Link: googleapis/googleapis@88f2ea3 Source-Link: https://github.com/googleapis/googleapis-gen/commit/230a5588306aae18fe8f2a57f14d4039ad72c901 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMjMwYTU1ODgzMDZhYWUxOGZlOGYyYTU3ZjE0ZDQwMzlhZDcyYzkwMSJ9
The https://github.com/GoogleCloudPlatform/jetty-runtime needs to be able to generate stackdriver log messages with the label
traceidset the the value obtained from thex-cloud-trace-contextHTTP header.However, the LoggingHandler#getLogEntry method does not allow for easy extension to support traceid as the method is private rather than protected, thus to change behaviour more methods on the class need to be replaced to customize the LogEntry.
If this method could be made protected, then it would be easy to extend the handler to use a threadlocal label map to set additional labels (eg traceid).
Better yet, such a threadlocal context could be implemented as a standard feature of the LogHandler?
The text was updated successfully, but these errors were encountered: