Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Don Bourne
    @donbourne
    ok, discussed with @Emily-Jiang -- per the new process, looks like we are covered -- see https://groups.google.com/forum/#!topic/microprofile/anNBKwk07Q8
    David Chan
    @Channyboy
    It's been updated to be a separate module now!
    And have incorporated your changes @jmartisk
    David Chan
    @Channyboy
    and may have addressed your application port issue
    Jan Martiška
    @jmartisk
    @Channyboy let's get this over with, you ok if I merge it and do RC2?
    Jan Martiška
    @jmartisk
    Suppose you're not here anymore :) I will do it
    David Chan
    @Channyboy
    Hey @jmartisk for liberty it's not tracking requests with unmapped exceptions
    Andy McCright
    @andymc12
    for unhandled exceptions, I'd suggest leaving it as optional/unspecified in the spec. I think it is a good idea, but not critical for the release.
    Jan Martiška
    @jmartisk
    maybe, but the problem then might be that the behavior depends on the JAX-RS implementation - some might call the response filters, some might not.. so basically the results of the measurements will depend on the underlying JAX-RS implementation
    Andy McCright
    @andymc12
    hmm... iiuc, the filter should be invoked even on errors - so is the problem that the stats might get skewed due to unhandled exceptions? i.e. normal requests take ~200ms, but some exceptions might cause it to take ~10ms, so the overall measurement looks lower than it should? (and that the user doesn't see potential app errors from the stats?)
    Don Bourne
    @donbourne
    @andymc12 I was thinking same thing... given we aren't separately labeling the metrics that were accumulated from jax-rs requests that end in exceptions we are possibly tainting our data
    so bigger question is -- should we track the duration/count of jax-rs requests that end in exceptions at all for 2.3? I think the right answer, eventually, is that we need a label to differentiate the exception metrics from the "clean path" metrics
    but it's almost an anti-pattern to track timing for exceptional and clean requests in the same time series
    and given the time remaining that somewhat leads me to think we should just discard time/count measurements for JAX-RS requests that end in exceptions... and put that in the spec. Then for 2.4 / 3.0 we can add a proper metric (with label) to count/time the exception paths
    Andy McCright
    @andymc12

    part of me thinks that tracking the exception cases is acceptable. the app could handle the exception and convert it to a response using an ExceptionMapper - in that case, they could throw a WidgetNotFoundException right off the bat that gets converted to a 404 response, and that would also skew the metrics, but is handled.

    but it's almost an anti-pattern to track timing for exceptional and clean requests in the same time series

    agreed - it's just nice to know if a large portion of the requests end in unexpected errors

    and given the time remaining that somewhat leads me to think we should just discard time/count measurements for JAX-RS requests that end in exceptions... and put that in the spec.

    I think that gets tricky because of the exception mapping in JAX-RS. some third-party libraries (even MP OpenTracing) registers global ExceptionMapper<Throwable> that catches all unhandled exceptions and converts them to "nicer" responses.

    Don Bourne
    @donbourne
    tracking the exception cases is acceptable -- but it's harder to say that mixing the counts/times of exceptions in with the clean path (and not giving a way to filter out the exceptions) makes sense
    Andy McCright
    @andymc12
    I agree - it just makes it hard for an implementor - like Jan said, the Metrics implementation will have different behavior (or might just not work) depending on the JAX-RS implementation.
    Don Bourne
    @donbourne

    I think that gets tricky because of the exception mapping in JAX-RS

    but are we able to tell when a request has had to go to an ExceptionMapper? could we just not record those?

    Andy McCright
    @andymc12
    I don't think we could tell that. The ExceptionMappers are application code - and there is no standard mechanism for notifying when an ExceptionMapper is invoked
    Don Bourne
    @donbourne
    brb
    Jan Martiška
    @jmartisk
    I'd say we perhaps leave it unspecified for now then. In the next version we can start adding a tag to the SimpleTimer depending on the http response code, or something? Let's not do major changes right now, it's too late. We need a quick and painless solution.
    Don Bourne
    @donbourne
    yeah, I was about to say we should possibly plan, in future, to add a response code label
    @andymc12 , does that make sense to you?
    @jmartisk , what part do we want to leave unspecified? Are we basically going to ignore the topic of JAX-RS exceptions in the spec/TCK in 2.3?
    Don Bourne
    @donbourne
    with intent that, in next release, we add specific wording about how mapped and unmapped exceptions should be handled, with corresponding TCK tests?
    and likely add labels for http response codes in next release as well?
    Jan Martiška
    @jmartisk
    Yeah that sounds good to me.
    Don Bourne
    @donbourne
    that's ok approach with me too... @andymc12 / @Channyboy ?
    David Chan
    @Channyboy
    sounds good
    Don Bourne
    @donbourne
    ok, I'll put something on our issue list for that so we can come back to it
    Andy McCright
    @andymc12
    (sorry, ran to a meeting) - yes, that sounds good to me too. Thanks!
    Jim Krueger
    @jim-krueger
    Joining this conversation late. FYI: The current behavior in Liberty is to collect and record mbean stats and metrics for all resource method invocations that are handled by the ContainerResponseFilter. ContainerResponseFilters are triggered for all Exceptions (both checked and unchecked) that are handled by an ExceptionMapper. All unmapped exceptions will cause the filter to be bypassed and therefore no metrics / mbean stats will be collected.
    Jan Martiška
    @jmartisk
    I made some comments on @Channyboy 's PR eclipse/microprofile-metrics#534 I suppose, that perhaps, hopefully, maybe, that's the last thing we need to do for RC3 and Final
    Don Bourne
    @donbourne
    yup, saw your comments (had noticed same thing about the REST bullet). Was looking at schedule yesterday in the MP community meeting -- I think we're well ahead of sched -- component spec/api/tck's are due Feb 11.
    Jan Martiška
    @jmartisk
    Glad to hear that :)
    David Chan
    @Channyboy
    Here's the build for the final release
    https://ci.eclipse.org/microprofile/view/Release/job/MicroProfile%20Releases/338/
    not dropped yet
    Jan Martiška
    @jmartisk
    Giving it a test run, looks good so far.
    Jan Martiška
    @jmartisk
    Looks good to me!
    Jan Martiška
    @jmartisk
    Hope there will be no more issues coming up at last minute. From my side it's a go for release
    Emily Jiang
    @Emily-Jiang
    Hi folks, you need to update the releases page to populate info properly
    For 2.3 release, you need do what Don has done for 2.2 https://github.com/eclipse/microprofile-metrics/releases/tag/2.2
    David Chan
    @Channyboy
    Hi @Emily-Jiang dropping the repo now and will do those steps now
    Emily Jiang
    @Emily-Jiang
    thanks @Channyboy
    Jan Martiška
    @jmartisk
    looks like we're live :)
    Jan Martiška
    @jmartisk
    Are you planning to send an announcement to the mailing list now that it's out? Or should I?
    Jan Martiška
    @jmartisk
    I think we should announce it
    Jan Martiška
    @jmartisk
    btw I just created the 2.3.x branch for micro updates to 2.3
    David Chan
    @Channyboy
    ah yes right, made the announcement now