[Essentials] (client) Error Telemetry logging: JEP in early draft review

[Baptiste Mathus]

Hello everyone,

So as a followup on the recent discussions around telemetry logging for Essentials, I've drafted a JEP on this matter I would love to get feedback on:

https://github.com/batmat/jep/tree/JENKINS-49805/jep/0000

Thanks!

-- Baptiste

[Jesse Glick]

Seems reasonable.

The implementation separates JSON records with a newline. This is not
apparent from the JEP; it would be nice to mention it, so that readers
do not wonder whether you are using some kind of streaming JSON parser
(which is unnecessary when each line is a well-formed JSON document).

`JsonFormatter.formatException` can be replaced with
`Functions.printThrowable`, which formats chained exceptions more
readably. The JEP proposal for a future `stacktrace` field neglects to
mention that this will not work as is for exceptions with causes
and/or suppressed exceptions. In general `Throwable.printStackTrace`
can be overridden in nontrivial ways, sometimes including information
not present in the `message` nor stack trace (though less so after
JENKINS-46140), so I would not recommend bothering with anything but a
plain printed string.

[Baptiste Mathus]

2018-04-05 0:53 GMT+02:00 Jesse Glick <jgl...@cloudbees.com>:

Seems reasonable.

The implementation separates JSON records with a newline. This is not
apparent from the JEP; it would be nice to mention it, so that readers
do not wonder whether you are using some kind of streaming JSON parser
(which is unnecessary when each line is a well-formed JSON document).

Done.

 

`JsonFormatter.formatException` can be replaced with
`Functions.printThrowable`, which formats chained exceptions more
readably. The JEP proposal for a future `stacktrace` field neglects to
mention that this will not work as is for exceptions with causes
and/or suppressed exceptions. In general `Throwable.printStackTrace`
can be overridden in nontrivial ways, sometimes including information
not present in the `message` nor stack trace (though less so after
JENKINS-46140), so I would not recommend bothering with anything but a
plain printed string.

Ack. Adjusted the proposal too, thanks a lot.

I will submit the PR to official JEP later today I think. Still giving it a few hours for some more feedback but I don't expect much pushback in this area and for such a small JEP anyway.

Cheers

[Baptiste Mathus]

Submitted as https://github.com/jenkinsci/jep/pull/77

Thanks everyone for the feedback.

[Baptiste Mathus]

Quick heads-up everyone. So since last email, the proposal got accepted as JEP-304.

After a discussion yesterday with Tyler, we discussed a small adjustment to make JSON parsing and evolution more straightforward, making the `exception` field an object instead of a string as it was currently.

Feel free to provide feedback here or on the PR.

https://github.com/jenkinsci/jep/pull/105

Thanks!