Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Request to alter Cloud Debugger JSON format #289

Closed
joakime opened this issue Jun 14, 2016 · 2 comments
Closed

Request to alter Cloud Debugger JSON format #289

joakime opened this issue Jun 14, 2016 · 2 comments

Comments

@joakime
Copy link

joakime commented Jun 14, 2016

How flexible are we in altering the JSON that the Cloud Debugger uses?

The current format being produced loses information (bad java.util.logging assumptions / faults).

Some concerns:

  • the conjoined "message" key has ..
    • logger name (unless caller source is declared)
    • caller source class no method (logger name is lost!)
    • caller source class and method (logger name is lost!)
    • throwable (multi-line throwable messages can cause issues)
  • throwable common / overlapping entries are not stripped (like other frameworks do)
  • thread name is assumed to be Thread.currentThread() at time of logging, not when the logging event was created
  • mdc (aka: LogContext) can overwrite json keys

An example output as it exists now ...

Note: added indents / pretty print to show it cleaner here, in practice the entry exists as a "one liner" (with exception of throwables)

{
  "timestamp": {
    "seconds": 12345,
    "nanos": 678000000
  },
  "severity": "INFO",
  "thread": "main",
  "message": "logger: message",
  "traceId": "abcdef"
}

In java.util.logging, the caller source class and method only exists when you use the Log.logp() style methods (which is in rare form).
The logger name should never be excluded is often more useful then the caller class + method (and wont confuse people attempting to configure logging levels)

The "message" entry also has the throwable appended onto the end of it, which in the case of multi-line throwable messages can be difficult to understand where the logging message ends and the exception message begins.

I propose the following changes:

  • keep "severity" (possible values "TRACE", "DEBUG", "INFO", "WARN", "ERROR". as this is the common overlap in all of the logging frameworks)
  • keep "timestamp" as-is, but based on logging event time
  • keep "thread" but base it on the thread at the time of the logging event creation
  • keep "message" but it is ONLY the formatted/parsed message for the log event
  • new (required) "logger" which is the logger name
  • new (optional) "caller" entry which is a string which has the class + method + extra
  • new (optional) "throwable" entry which has the entire stacktrace, sans duplicate stacktrace elements
  • optional "traceId" is always in top level (and obtained from MDC)
  • all other MDC entries are in a new "mdc" sub-object

This will make the json compatible with more then java.util.logging events, and can be populated with events from log4j, commons-logging, java.util.logging, slf4j, logback, logkit, osgi, and apache-juli.

Note: the "throwable" section can be formatted to suite the Cloud Debugger team in any fashion they want. The "caller" entry is often exposed by other (not java.uitil.logging) logging frameworks as a full call stack, not just a single class + method (we can expose this stack if they deem it useful, or just default to the first entry)

@vlad-lifliand
Copy link

Could you please elaborate on the scenario? Are you reading Cloud Debugger generated logpoints using AppEngine logging API?

Cloud Debugger uses logp() function internally, so the output is not as flexible as we would like it to be. The relevant source code is here: https://github.com/GoogleCloudPlatform/cloud-debug-java/blob/master/src/agent/jvm_dynamic_logger.cc#L89.

@joakime
Copy link
Author

joakime commented Jun 23, 2016

The effort to modernize the logging layer is being scaled back (result of some discussions), as it will cause other issues in the compat layer. I'm considering this closed.

@joakime joakime closed this as completed Jun 23, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants