Add evaluateAt support for probe instrumentations #4320
Conversation
jpbempel
requested review from
cimi,
a team and
ojung
and removed request for
a team and
cimi
jpbempel
force-pushed
the
jpbempel/evaluate-at-usage
branch
from
92e7a17
to
281f012
Compare
| case EXIT: | ||
| return Snapshot.MethodLocation.EXIT; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Will this become a NPE in the caller?
|
|
||
| protected LabelNode getReturnHandler(AbstractInsnNode exitNode) { | ||
| // exit node must have been removed from the original instruction list | ||
| assert exitNode.getNext() == null && exitNode.getPrevious() == null; |
There was a problem hiding this comment.
Assertions aren't enabled by default, right?
There was a problem hiding this comment.
no, only if you enable with -ea
There was a problem hiding this comment.
Soo, this is redundant because nobody enables assertions?
There was a problem hiding this comment.
How about replacing with a IllegalArgumentException?
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected InsnList getBeforeReturnInsnList(AbstractInsnNode node) { | ||
| return callMetric(metricProbe); |
There was a problem hiding this comment.
would we be able to add condition for metrics in this fashion as well?
| @@ -290,6 +295,12 @@ public enum Kind { | |||
| AFTER; | |||
| } | |||
|
|
|||
| public enum MethodLocation { | |||
| NONE, | |||
There was a problem hiding this comment.
I find the name and value confusing.
evalautAt = None means evaluate both at entry and exit for snapshot but entry for metric and logs (I think). maybe change it to DEFAULT or use null to indicate there is nothing there.
Also suggest to rename to EvalauteAtLocation not confusing with method inside ProbleLocation.
* Snapshot: indicates for method probe where conditions are evaluated before serializing data. Either at entry or exit of the method. * Metric: indicates for method probe where the metric is emitted and with which values. For example at Exit of the method you can emit the return value of the method. * Log: use the same mechanism than snapshot but can report for example at exit of the method the return value.
jpbempel
force-pushed
the
jpbempel/evaluate-at-usage
branch
from
53f4bc4
to
3e0b9c9
Compare
jpbempel
force-pushed
the
jpbempel/evaluate-at-usage
branch
from
3e0b9c9
to
b184e07
Compare
What Does This Do
Motivation
following #4313
Additional Notes