Re: RFR: 8072645: java.util.logging should use java.time to get more precise time stamps

[Daniel Fuchs]

Hi Stephen,

Thanks a lot for your feedback, it is really helpful!
On 2/14/15 11:28 AM, Stephen Colebourne wrote:
Elements of the date/time handling here don't really work.

Logging is essentially a UTC only problem, using a time-zone is slow and
unecessary. This indicates that all uses of ZonedDateTime should be
replaced with either Instant or an OffsetDateTime using ZoneOffset.UTC.

Any string format should have the ISO-8601 string end with "Z", and not end
with "-05:00" or any other zone offset.

If I'm not mistaken the previous SimpleFormatter used to use java.util.Date
and printed the time in the local time zone. I have tried to keep this 
behavior.
I'm not sure we would want to change it to print the time in the UTC 
time zone
by default. A lot of developers use logging for debugging - and when reading
debug messages on the console I usually prefer to see the time in my own
time zone.

Would there be a more efficient way to keep the default formatting of 
the time
in the SimpleFormatter?

(The webrev is broken wrt zones as it stores ZoneId.systemDefault() in a
static constant, but that method depends on the mutable
TimeZone.getDefault() ).

Would making it a final (non static) variable be better?
I now wonder whether there should be a way to configure the time zone for
an instance of SimpleFormatter (something like what I did with 'printNanos'
for the XMLFormatter).

LogReecord.getInstantUTC() should be getInstant().

(An Instant is fully defined as a concept, and it cannot be in or not in
UTC).

Right. Thanks for pointing that out.
In SimpleFormatter, you have {@code java.util.loggin} (missing a "g").

Good catch.

In XMLFormatter, instead of using DateTimeFormatter.ISO_LOCAL_DATE_TIME,
create a new instance of DateTimeFormatter that does not output the
fraction of a second. That way, there is no need to use
truncatedTo(SECONDS).

StringBuilder appends can be used directly with formatters:

sb.append(ZonedDateTime.ofInstant(time, zoneId).format(dtformatter));

replace with

dtformatter.formatTo(ZonedDateTime.ofInstant(time, zoneId), sb);

Will look into this.

Thanks again for your review! This is quite helpful.

-- daniel



thanks
Stephen



On 13 Feb 2015 14:57, "Daniel Fuchs" <daniel.fu...@oracle.com> wrote:

Hi,

Please find below a patch for:

8072645: java.util.logging should use java.time to get more
          precise time stamps


http://cr.openjdk.java.net/~dfuchs/webrev_8072645/webrev.00/

specdiff:
http://cr.openjdk.java.net/~dfuchs/webrev_8072645/
specdiff-logging-time/java/util/logging/package-summary.html

Overview:
---------

The patch is made of the following pieces:

  - LogRecord uses java.time.Clock's systemClock to get an
    Instant in the best available resolution.

    The instant is split into a number of milliseconds (a long)
    and a nanosecond adjustment (an int).
    The number of milliseconds is the same than what would have
    been obtained by calling System.currentTimeMillis().

  - LogRecord acquires a new serializable int nanoAdjustement field,
    which can be used together with the number of milliseconds
    to reconstruct the instant.

  - SimpleFormatter is updated to pass a ZoneDateTime
    instance to String.format, instead of a Date.

    The effect of that is that the format string can now
    be configure to print the full instant precision, if
    needed.

  - XMLformatter will add a new <nanos> element after the
    <millis> element - if the value of the nanoAdjustment
    field is not 0.

    The <date> string will also contain the nano second
    adjustment as well as the zone offset as formatted by
    DateTimeFormatter.ISO_OFFSET_DATE_TIME

Compatibility considerations:
-----------------------------

- The serial for of log record is backward/forward compatible.
   I added a test to verify that.

- XMLFormatter has acquired a new configurable property
   '<FQCN>.printNanos' which allows to revert to the old
   XML format, should the new format cause issues in
   existing applications.

- The logger.dtd will need to be updated, to support the
   new optional <nanos> element. And for this matter,
   should we update the logger.dtd or rather define a
   logger-v2.dtd?
   See planned modification:

<http://cr.openjdk.java.net/~dfuchs/webrev_8072645/logger-
dtd/logger.dtd.frames.html>

best regards,

-- daniel