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
> >>>>
> >>
>

Reply via email to