That's the reason to skip calling #inc() and #dec() if there is no MetricRegistry in the ServletContext.
Static is evil ;-) Try to avoid it as much as possible. Despite being Apache Tomcat committer my only application that uses Wicket 8.0.0-SNAPSHOT runs on Jetty :-) I'm too lazy to setup development environment with Tomcat for now. On Mon, Mar 28, 2016 at 2:43 PM, Tobias Soloschenko < [email protected]> wrote: > Have you tested the startup order of the classes? AOP is started before > the webapp starts and with it the listener. So if tomcat restore a session > - the new constructor of some components can be invoked and then the setup > fails because there might be aspects around it. > > So check if the listener is started before any getSettings() or > getRegistry() is invoked. Even if sessions are restored after server start. > > That was the original reason why I used statics. > > kind regards > > Tobias > > > Am 28.03.2016 um 14:00 schrieb Martin Grigorov <[email protected]>: > > > > I am not sure what you mean. > > > > SessionCountListener uses @WebListener so it will be automatically added > to > > the application if the metadata is not complete. > > If <web-app metadata-complete="true"> then the application developer will > > have to add a <listener> manually if (s)he wants to use it. > > > > SessionCountListener#inc() and #dec() methods will be called only if > there > > is MetricRegistry in the ServletContext, i.e. only if wicket-metrics is > in > > the classpath because org.apache.wicket.metrics.Initializer takes care to > > set it up. > > > > As I said in my previous mail the static and #get(String) are not needed > by > > any of the current aspects, so I think it is OK to remove them. We can > > reintroduce them if someone comes with a use case that is not supported > by > > the threadlocal and/or the servlet context. > > > > Martin Grigorov > > Wicket Training and Consulting > > https://twitter.com/mtgrigorov > > > > On Mon, Mar 28, 2016 at 12:51 PM, Tobias Soloschenko < > > [email protected]> wrote: > > > >> So the Listener is required, now? Or is this only an option if you have > >> more then one application in one war? > >> > >> I created the SessionCountListener only as a helper for the aspect to > >> count sessions. Now it is used to store the session / the registry. > >> > >> About the statics and get(String)? Can they he removed? > >> > >> kind regards > >> > >> Tobias > >> > >>> Am 28.03.2016 um 12:40 schrieb Martin Grigorov <[email protected]>: > >>> > >>> I've pushed the second part. > >>> Now SessionCountListener and SessionCountListenerAspect use the > >>> MetricRegistry and WicketMetricsSettings stored in the ServletContext. > >>> > >>> For applications which live in the same .war file the registry and the > >>> settings are shared by default. Each application can override any of > >> those > >>> in MyApp#init() method > >>> Applications in separate .war files will have different registry and > >>> settings. > >>> > >>> I don't see a need to use <env-entry> for any use case now. > >>> The other ways to lookup the registry and the settings could stay, > >> although > >>> all current aspects could work by using the ThreadLocal or the > >>> ServletContext. > >>> > >>> Martin Grigorov > >>> Wicket Training and Consulting > >>> https://twitter.com/mtgrigorov > >>> > >>> On Mon, Mar 28, 2016 at 10:10 AM, Tobias Soloschenko < > >>> [email protected]> wrote: > >>> > >>>> Mhhh the SessionCountListener is optional - keep that in mind - if a > >>>> Session is restored this might cause issues but I am looking forward > to > >> see > >>>> the final changes. > >>>> > >>>> kind regards > >>>> > >>>> Tobias > >>>> > >>>>> Am 28.03.2016 um 09:28 schrieb Martin Grigorov <[email protected] > >: > >>>>> > >>>>> I've pushed a commit that I believe will help. > >>>>> I have to go out now, so I'll finish it in the afternoon. > >>>>> > >>>>> Martin Grigorov > >>>>> Wicket Training and Consulting > >>>>> https://twitter.com/mtgrigorov > >>>>> > >>>>> On Mon, Mar 28, 2016 at 9:25 AM, Tobias Soloschenko < > >>>>> [email protected]> wrote: > >>>>> > >>>>>> Hi Martin, > >>>>>> > >>>>>> thank you! > >>>>>> > >>>>>> kind regards > >>>>>> > >>>>>> Tobias > >>>>>> > >>>>>>> Am 28.03.2016 um 09:15 schrieb Martin Grigorov < > [email protected] > >>> : > >>>>>>> > >>>>>>> Hi Tobias, > >>>>>>> > >>>>>>> I haven't tested it with two .war files yet but I can see how to > >> break > >>>>>> this > >>>>>>> approach too: add two Wicket applications in the same .war, i.e. > two > >>>>>>> <filter>s for two different applications. > >>>>>>> The env-entry is shared for all servlets/filters in the .war. > >>>>>>> > >>>>>>> I'll try to rework it to use the ServletContext now. > >>>>>>> > >>>>>>> Martin Grigorov > >>>>>>> Wicket Training and Consulting > >>>>>>> https://twitter.com/mtgrigorov > >>>>>>> > >>>>>>> On Mon, Mar 28, 2016 at 7:51 AM, Tobias Soloschenko < > >>>>>>> [email protected]> wrote: > >>>>>>> > >>>>>>>> Update: > >>>>>>>> > >>>>>>>> I finally got it working for me: > >> > https://github.com/klopfdreh/wicket/commit/dda71c9e5ce5135993df0ea85450d232f14e53c5 > >>>>>>>> > >>>>>>>> @Martin: Would be able to also test it with two or more > applications > >>>>>>>> running on the same server? > >>>>>>>> > >>>>>>>> In your MetricsServlet.ContextListener you could easily write > >>>>>>>> WicketMetrics.getMetricsRegistry() - The detection which > application > >>>>>> should > >>>>>>>> be shipped is handled if you set up environment entries in each > >>>> project. > >>>>>>>> (see commit) > >> > https://dropwizard.github.io/metrics/3.1.0/manual/servlets/#manual-servlets > >>>>>>>> > >>>>>>>> kind regards > >>>>>>>> > >>>>>>>> Tobias > >>>>>>>> > >>>>>>>> Am 27.03.16 um 13:12 schrieb Martin Grigorov: > >>>>>>>> > >>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> The problem with -Dwicket.metrics.applicationName and the static > >>>>>> variables > >>>>>>>>> approach is that both cannot be used if you have two Wicket > >>>>>> applications > >>>>>>>>> in > >>>>>>>>> the web server. > >>>>>>>>> > >>>>>>>>> There is no need to implement a custom Servlet. metrics-servlet > >>>> already > >>>>>>>>> provides it. We just have to make the MetricsRegistry available > in > >>>> the > >>>>>>>>> ServletContext (attribute > >>>>>>>>> "com.codahale.metrics.servlets.MetricsServlet.registry") > >>>>>>>>> > >>>>>>>>> Martin Grigorov > >>>>>>>>> Wicket Training and Consulting > >>>>>>>>> https://twitter.com/mtgrigorov > >>>>>>>>> > >>>>>>>>> On Sat, Mar 26, 2016 at 2:34 AM, Tobias Soloschenko < > >>>>>>>>> [email protected]> wrote: > >>>>>>>>> > >>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>> I would prefer to stay by the Application.get(String) / > >>>>>> Application.get() > >>>>>>>>>> / static because it is more Independent and not bound to the > >> webapp > >>>>>>>>>> lifecycle. Currently we only rely on the servlet stuff in one > >>>> metric - > >>>>>>>>>> which is at least not required. Even in Wicket itself you should > >> not > >>>>>>>>>> access > >>>>>>>>>> the HttpSession itself. > >>>>>>>>>> > >>>>>>>>>> But this is only my opinion - let us hear other suggestions. > >>>>>>>>>> > >>>>>>>>>> Maybe it is a good idea to implement a defaul servlet which can > be > >>>>>>>>>> configured and exposes the metrics / registry via http get - > which > >>>>>> uses > >>>>>>>>>> the > >>>>>>>>>> dropwizard metric servlet. > >>>>>>>>>> > >>>>>>>>>> kind regards > >>>>>>>>>> > >>>>>>>>>> Tobias > >>>>>>>>>> > >>>>>>>>>>> Am 25.03.2016 um 22:51 schrieb Martin Grigorov < > >>>> [email protected] > >>>>>>> : > >>>>>>>>>>> > >>>>>>>>>>> Just moments after sending the mail I recalled that DropWizard > >>>>>> provides > >>>>>>>>>>> something similar: > >>>>>>>>>>> https://dropwizard.github.io/metrics/3.1.0/manual/servlets/ > >>>>>>>>>>> So there is no need of a custom IResource. > >>>>>>>>>>> We just have to make it easier to lookup the MetricsRegistry > from > >>>>>>>>>>> MetricsServlet - via the ServletContext. > >>>>>>>>>>> > >>>>>>>>>>> I wonder whether the ServletContext solution could be used > >> instead > >>>> of > >>>>>>>>>>> the > >>>>>>>>>>> Application#get(String) and static variable fallbacks. I.e. > >> somehow > >>>>>> to > >>>>>>>>>> get > >>>>>>>>>> > >>>>>>>>>>> a reference to ServletContext in Session#onInvalidate(). > >>>>>>>>>>> > >>>>>>>>>>> Martin Grigorov > >>>>>>>>>>> Wicket Training and Consulting > >>>>>>>>>>> https://twitter.com/mtgrigorov > >>>>>>>>>>> > >>>>>>>>>>> On Fri, Mar 25, 2016 at 10:48 PM, Martin Grigorov < > >>>>>> [email protected] > >>>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>> Hi Tobias, > >>>>>>>>>>>> > >>>>>>>>>>>> Inspired by > >> https://github.com/jooby-project/jooby/tree/master/jooby-metrics > >>>> I > >>>>>>>>>>> think > >>>>>>>>>> > >>>>>>>>>>> it would be nice if wicket-metrics provides a IResource that > >>>> renders > >>>>>>>>>>> JSON > >>>>>>>>>> > >>>>>>>>>>> with the current metrics per type/aspect. > >>>>>>>>>>>> I.e. if /wicket/metrics/ is requested then it dumps something > >>>> like: > >>>>>>>>>>>> > >>>>>>>>>>>> { > >>>>>>>>>>>> "SomeTimerAspect" : {min:.., max:..., mean:..., ...}, > >>>>>>>>>>>> ... > >>>>>>>>>>>> "SomeCounterAspect" : {value:..}, > >>>>>>>>>>>> ... > >>>>>>>>>>>> } > >>>>>>>>>>>> > >>>>>>>>>>>> When /wicket/metrics/SomeCounterAspect is requested then : > >>>>>>>>>>>> {"value": ...} > >>>>>>>>>>>> is rendered. > >>>>>>>>>>>> > >>>>>>>>>>>> Do you think it is a good idea ? > >>>>>>>>>>>> > >>>>>>>>>>>> It will be useful for quicker checks of the current state. > >>>>>>>>>>>> > >>>>>>>>>>>> The application will have to mount it explicitly in > >> MyApp#init(). > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> Martin Grigorov > >>>>>>>>>>>> Wicket Training and Consulting > >>>>>>>>>>>> https://twitter.com/mtgrigorov > >> >
