You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by klopfdreh <gi...@git.apache.org> on 2016/03/12 23:47:39 UTC
[GitHub] wicket pull request: wicket-metrics
GitHub user klopfdreh opened a pull request:
https://github.com/apache/wicket/pull/164
wicket-metrics
Just a PR to make the review easier.
The implementation was one of the ideas mentioned in the confluence page: https://cwiki.apache.org/confluence/display/WICKET/Ideas+for+Wicket+8.0
Ticket has to be created
@martin-g - Is there a ticket for the metrics topic already?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/klopfdreh/wicket wicket_metrics
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/164.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #164
----
commit 7ef05feadde89b2554576ee50f9ce30d56a552ae
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-10T10:40:39Z
Wicket Metrics
commit 399c08eb14241588f1c98a4d3614e1edbfd86037
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-10T12:27:22Z
Wicket Metrics - Fixed errors on startup
commit 0f0f6b0e6bcc93f3655b8825ad02e95c95145a03
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-10T13:03:19Z
Wicket Metrics - Time measurement / license header
commit 756612158e391d43155a3b1373a29bf5a789dcce
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-10T13:12:21Z
Wicket Metrics - Simplified time measurement
commit f7769716545f77aff6ce47cb7b89494d80cc5aa7
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-10T14:12:33Z
Wicket Metrics - Component create / render / ResourceReference create
commit 5cb7b27b4dc1bf295737fd3ecb8b3547144a1651
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-10T14:16:38Z
Wicket Metrics - Switched create to time measure
commit c50e3040adc561cc5ac3b57cb11e4efd2766e86b
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-11T18:29:30Z
Wicket Metrics - Guide / Added several new measurement aspects
commit b8ebc68a06e0c38ebc87a17cd4ae174d3a2c90b8
Author: Tobias Soloschenko <ts...@apache.org>
Date: 2016-03-12T08:05:28Z
Wicket Metrics - Screenshot / further guide improvements
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933641
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
--- End diff --
`mark` is a bit confusing. Why keep it `meter`? To be the same as what MetrictsRegistry uses.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934768
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
--- End diff --
done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r56370130
--- Diff: wicket-experimental/wicket-metrics/src/main/resources/wicket-metrics.template.xml ---
@@ -0,0 +1,26 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+<aspectj>
+ <weaver options="-nowarn">
+ <include within="org.apache.wicket..*"/>
+ <include within="com.fluentsqaure..*"/>
--- End diff --
Thank you for reviewing!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-197423520
Integrated
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-196058123
Ticket is created: https://issues.apache.org/jira/browse/WICKET-6120
I choose this way of API because it actually not able to configure metrics from start (if available in the class path), because AspectJ is started before the init method is executed. But you can deactivate the JMX functionality and the measurement with meta data keys:
```java
WicketMetricsSettings settings =
Application.get().getMetaData(WicketMetrics.METRIC_SETTINGS);
settings.startJmxReporter();
settings.stopJmxReporter();
settings.setEnable(bool);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933626
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
--- End diff --
This needs synchronization.
Or better - initialize the registry in IInitializer#init()
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934729
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onConfigure
+ *
+ * @param joinPoint
+ * the join point (component) which is configured
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onConfigure
+ */
+ @Around("execution(* org.apache.wicket.Component.onConfigure(..))")
+ public Object aroundOnConfigure(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/configure", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onInitialize
+ *
+ * @param joinPoint
+ * the join point (component) which is initialized
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onInitialize
+ */
+ @Around("execution(* org.apache.wicket.Component.onInitialize(..))")
+ public Object aroundOnInitialize(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/initialize", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onDetach
+ *
+ * @param joinPoint
+ * the join point (component) which is calling detach
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while invoking onDetach
+ */
+ @Around("execution(* org.apache.wicket.Component.onDetach(..))")
+ public Object arroundOnDetach(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return mark("core/component/detach", joinPoint);
--- End diff --
how you will know that the call is "manual" ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934696
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/IPartialPageRequestHandlerAspect.java ---
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Aspect which checks ajax request targets
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+@Aspect
+public class IPartialPageRequestHandlerAspect extends WicketMetrics
--- End diff --
This metric is used to collect data how often components have been added for repaint. Though this is a useful information.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-195912054
No there is no ticket yet.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217837350
> Strange, because AspectJ manipulates/instruments classes with a behavior which is not needed in a system which do not use metrics. AspectJ and javassist use the same way to operate: instrumenting of code.
If you let away any AspectJ jars and the dependency to wicket-metrics - wicket-core behaves like before. If you add the AspectJ jars and the dependency to wicket-metrics - the aspects are weaved around the method / constructor calls and the metrics are created. (This is an advantage of the agent usage)
> There is not need to compile core classes: we are profiling custom app and not core. So the main target for instrumenting, should be custom code, but not core themselves. There is quite thin boundary: if you check my code - it's dynamically overrides core methods in a custom class. So even pure Page's core methods can be profiled.
That is not right in general. Imagine the circumstances that the user has a lot of AjaxLinks and override the onClick mechanism within an anonymous inner class. The code is implemented by a customer, but the class is provided by wicket-core and because of this also core classes would be needed to be manipulate.
Also if you want to measure the request time, you have to consolidate core classes.
> And one more point: I wouldn't say that injection is key point for profiler. Last one should be transparent for application's logic. So profiler should not change application's fields, arguments and etc. It should be one level up.
That is not a good idea. One requirement was to log the actual request which reached the server (also with url):
https://github.com/apache/wicket/blob/master/wicket-experimental/wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/request/WicketFilterRequestCycleUrlAspect.java#L54
This would not be possible anymore if you do it the way you suggested.
> Btw, I have question regarding AspectJ implementation: java agents has ability to be attached remotely to a working java processes. http://docs.oracle.com/javase/6/docs/technotes/guides/attach/index.html
> Is it a case for AspectJ? If it's a case: using agent become actually a good pro.
I didn't had a look at the attach API yet, but it would be a great advantage if you could setup your own "metrics logging server" indeed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934780
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
--- End diff --
Mh I think just only a boolean fits it - it is only used to disable it during runtime
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934804
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ getMetricRegistry().meter(PREFIX + name + renderClassName(joinPoint)).mark();
+ }
+ if (joinPoint != null)
+ {
+ return joinPoint.proceed();
+ }
+ return null;
+ }
+
+ /**
+ * Stops the contex quietly
+ *
+ * @param context
+ * the context to stop
+ */
+ public void stopQuietly(Context context)
+ {
+ if (context != null)
+ {
+ context.stop();
+ }
+ }
+
+ /**
+ * Starts the jmx reporter
+ */
+ public static void startJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().start();
+ }
+
+ /**
+ * Stops the jmx reporter
+ */
+ public static void stopJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().stop();
+ }
+
+ /**
+ * If the metrics should be enabled
+ *
+ * @param enabled
+ * if the metrics should be enabled
+ */
+ public static void setEnabled(boolean enabled)
+ {
+ WicketMetrics.enabled = enabled;
--- End diff --
Good point - I will change it this way!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-196168455
Something got broken, because it is not possible to ensure that Application is set before the LTW of AspectJ processes the weaving. Have to find a way for this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934747
--- Diff: wicket-metrics/src/main/resources/META-INF/aop.xml ---
@@ -0,0 +1,13 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+<aspectj>
+ <weaver options="-nowarn">
+ <include within="org.apache.wicket..*"/>
+ </weaver>
+ <aspects>
+ <aspect name="org.apache.wicket.metrics.aspects.BehaviorAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.IPartialPageRequestHandlerAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.ComponentAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.WicketFilterAspect" />
--- End diff --
This answer combined with the one for my question on IPartialPageRequestHandlerAspect makes me think it would be better to not add any aspect by default.
Each application has to decide which ones to use for itself.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55943704
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
--- End diff --
I just tested it with non static setup - there is an issue because the configuration has to be at load time of the classes..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217828319
I little bit incorrect conclusions...
Regarding injections: it's possible to do in many interesting way - for example you can even change passed arguments to a method/constructor. Yes - there is no annotations to automate injections, but still it's extremely easy to do. All injectors use the same approach as available for javassist too: proxies, deep reflection and etc.
And one more point: I wouldn't say that injection is key point for profiler. Last one should be transparent for application's logic. So profiler should not change application's fields, arguments and etc. It should be one level up.
There is not need to compile core classes: we are profiling custom app and not core. So the main target for instrumenting, should be custom code, but not core themselves. There is quite thin boundary: if you check my code - it's dynamically overrides core methods in a custom class. So even pure Page's core methods can be profiled.
Regarding support of Component's: it's really depends on how IClassResolver is being used in wicket. If it will allow: it will be a good place for instrumenting anything from custom app logic.
The following statement is strange, imho:
> We don't want to manipulate or compile classes with a behavior which is not needed in a system which do not use metrics.
Strange, because AspectJ manipulates/instruments classes with a behavior which is not needed in a system which do not use metrics. AspectJ and javassist use the same way to operate: instrumenting of code.
Btw, I have question regarding AspectJ implementation: java agents has ability to be attached remotely to a working java processes. http://docs.oracle.com/javase/6/docs/technotes/guides/attach/index.html
Is it a case for AspectJ? If it's a case: using agent become actually a good pro.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933649
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ getMetricRegistry().meter(PREFIX + name + renderClassName(joinPoint)).mark();
+ }
+ if (joinPoint != null)
+ {
+ return joinPoint.proceed();
+ }
+ return null;
+ }
+
+ /**
+ * Stops the contex quietly
+ *
+ * @param context
+ * the context to stop
+ */
+ public void stopQuietly(Context context)
+ {
+ if (context != null)
+ {
+ context.stop();
+ }
+ }
+
+ /**
+ * Starts the jmx reporter
+ */
+ public static void startJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().start();
+ }
+
+ /**
+ * Stops the jmx reporter
+ */
+ public static void stopJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().stop();
--- End diff --
Does this stop the same reporter ?
I haven't looked at the implementation but it could be that it builds a new one that is immediately stopped.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217771898
I think a good PoC would be to actually replace AspectJ with Javassist in the metrics submodule and handle each functionality:
* Configuration of each metric
* Measurement even for class hierarchy like in the previous mentioned scenario
* Activation and deactivation of the metric so that it doesn't have any impact to the core code (I think this is not possible because you only can decide via flag if the metric is active / not active but the Byte Code is filtered anyway)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934807
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
--- End diff --
the weaving is only configured once and the measurement is really fast - I think if the user decides to use measurement he implement it otherwise not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934660
--- Diff: wicket-user-guide/src/docs/guide/monitoring/monitoring_1.gdoc ---
@@ -0,0 +1,19 @@
+This is a little example how to setup wicket-metrics within a tomcat.
--- End diff --
done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933608
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
--- End diff --
The usage of statics bothers me.
OSGi users most probably won't be able to use it.
It might lead to problems with deployments.
Maybe add a dependency to wicket-core and store the metrics in Application's metadata.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217773408
I think it would he great to not require an agent anymore that is why I am asking all those questions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217611343
But AspectJ is using exactly the same approach: instrumenting classes. So idea is brilliant, but it will be even more bright if no agent will be required to do this stuff. I think the same can be done through javassist and instrumenting within IClassResolver: so no agent will be needed and it might be turned on and off right on production if needed without bouncing of a server.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217790358
I just googled a bit about javassist and aspectj and a good page which describes the dis-/advantages of both frameworks is:
https://theholyjava.wordpress.com/2011/09/07/practical-introduction-into-code-injection-with-aspectj-javassist-and-java-proxy/
> Javassist has no tools to perform the injection and you thus have to implement your own injection code \u2013 including that there isn\u2019t support for injecting the code automatically based on a pattern
I think this may increase the work to do, because every single annotation provided by the different metrics have to be implemented in a programmatic way. GluonJ requires an Agent if you don't want to compile classes again and want to use some annotations to hook into.
So my point still exists:
If you don't want to compile the classes again - also core classes (which is actually a code manipulation and might influence the existing behavior) you have to use an agent and this is something we don't want to with metrics.
In your example you only used your own classes and not core classes like Component - I think you will not be able to apply the programmatic javassist metrics there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933643
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ getMetricRegistry().meter(PREFIX + name + renderClassName(joinPoint)).mark();
+ }
+ if (joinPoint != null)
+ {
+ return joinPoint.proceed();
+ }
+ return null;
+ }
+
+ /**
+ * Stops the contex quietly
--- End diff --
Typo
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r56366533
--- Diff: wicket-experimental/wicket-atmosphere/pom.xml ---
@@ -18,13 +18,12 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
- <groupId>org.apache.wicket.experimental.wicket7</groupId>
+ <groupId>org.apache.wicket.experimental.wicket8</groupId>
<artifactId>wicket-experimental</artifactId>
<version>8.0.0-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>
<artifactId>wicket-atmosphere</artifactId>
- <version>0.24-SNAPSHOT</version>
--- End diff --
The removal means that the version of the module will be 8.0.0-SNAPSHOT from now on.
It is intentionally 0.x to allow change in APIs because it is experimental.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh closed the pull request at:
https://github.com/apache/wicket/pull/164
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217642745
@PhantomYdn one issue here: the license
Javassist version 3
Copyright (C) 1999- by Shigeru Chiba, All rights reserved.
https://github.com/jboss-javassist/javassist
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934724
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
--- End diff --
Okay, will be changed if meta data is implemented.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934826
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
--- End diff --
OK, then move it to settings.
Avoid statics as hard as possible!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-196422828
Hi Martin,
I just added a lot of stuff I made yesterday. I split every measurement into a single class so that you can decide much better what you want to measure and what you don't want to.
In addition I added a guide how to define own metric aspects in the guide.
The last thing I've done is to move it to experimental and fixed the artifact id to be *wicket8 instead of *wicket7 - Is this project automatically released with wicket 8 core?
I also added a template so that you can see what to define if you want to have all stuff. Just open a resource in your ide called "wicket-metrics.template.xml" if you have added the maven dependency.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933827
--- Diff: wicket-metrics/src/main/resources/META-INF/aop.xml ---
@@ -0,0 +1,13 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+<aspectj>
+ <weaver options="-nowarn">
+ <include within="org.apache.wicket..*"/>
+ </weaver>
+ <aspects>
+ <aspect name="org.apache.wicket.metrics.aspects.BehaviorAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.IPartialPageRequestHandlerAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.ComponentAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.WicketFilterAspect" />
--- End diff --
I haven't reached to the docs yet but this would be useful information there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934893
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
--- End diff --
My idea is to reduce the noise and the memory consumption.
If many things are measured then my memory usage will grow and my JMX tree will be bigger.
But a I suggested we can make any aspect optional by not putting it in aop.xml by default. If the developer wants to measure just her application code then (s)he has to provide custom aspect with more specific rules.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934656
--- Diff: wicket-user-guide/src/docs/guide/monitoring/monitoring_3.gdoc ---
@@ -0,0 +1,18 @@
+The data which is going to be measured depends on the wicket-metrics implementation. So it doesn't make any sense to collect time data
+
+about setRepsonsePage, but it does for the constructor of components, to see if a component needs a long time to be created. You can
--- End diff --
fixed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217608441
Because AspectJ is the only way to also measure Information about constructing Java classes - for example if you want to measure time how long it takes to invoke the constructor - each time - not only for resolving.
Also it offeres way more possibilities to measure Information in various ways - have a look at the packages and the corresponding classes. Also a discussion has been started at the mailing list were some topics has been discussed.
http://apache-wicket.1842946.n4.nabble.com/wicket-metrics-statistics-for-wicket-td4673887.html
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934793
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ getMetricRegistry().meter(PREFIX + name + renderClassName(joinPoint)).mark();
+ }
+ if (joinPoint != null)
+ {
+ return joinPoint.proceed();
+ }
+ return null;
+ }
+
+ /**
+ * Stops the contex quietly
--- End diff --
done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934850
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
--- End diff --
I see below the `MetricsRegistry#meter` is used.
For me, as a user, it is not clear what `mark` would do. I have to check the code to find out.
If `meter` is used instead then it would be clear to me what it does (assuming I have some experience with Dropwizard-metrics library.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217621978
Have you had a look at the packages? So please give me an example how to measure:
* constructor invocation time
* initialization time
* render time
* detach time
without injecting code into the existing classes to ensure that if you turn off metrics it does not has any influence to the exisiting one by only using the IClassLoader mechanism.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217699016
I had only a short look at it and I have some questions:
1. How can I configure metrics - activate / deactivate them
2. What about the performance - you are working a lot with concurrent classes
3. Is there a way to issue separate classes for the metrics
4. is there a way to let the user configure the metrics (xml)
kind regards
Tobias
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217621759
The advantage of AspectJ in this case is that if you develop any new components the time is automatically measured by adding the corresponding aspect. You don't need to register it in any way.
It is well known that there are some restrictions and you have to use an agent but the benefit is bigger then the disadvantages in this scenrios.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217773488
Looking forward to See it and thanks a lot!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217661354
Hi,
ok then lets have a look if every metric can be ported.
Looking forward to see the impl. :-)
Thanks for your feedback!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217636415
@klopfdreh , first of all let me mention that I used javassist, but I haven't try yet to use IClassResolver. If we are talking just about javassist - it can definitely to exactly the same and even more rather that AspectJ - just because it's more low level in bytecode manipulations rather then AspectJ. But if we are talking about wicket IClassResolver capabilities: I don't have much experience in that, but according to sources it's being used for every requested page and assuming that requests have small amount of intersections - it looks pretty OK just in runtime instrument page for one request and do not instrument it for another.
How to measure. There is brief description: http://www.ibm.com/developerworks/library/j-dyn0916/
http://jboss-javassist.github.io/javassist/tutorial/tutorial.html
Let me actually try to implement what I'm saying and comeback with some PoC. It will be much more fair:)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934801
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ getMetricRegistry().meter(PREFIX + name + renderClassName(joinPoint)).mark();
+ }
+ if (joinPoint != null)
+ {
+ return joinPoint.proceed();
+ }
+ return null;
+ }
+
+ /**
+ * Stops the contex quietly
+ *
+ * @param context
+ * the context to stop
+ */
+ public void stopQuietly(Context context)
+ {
+ if (context != null)
+ {
+ context.stop();
+ }
+ }
+
+ /**
+ * Starts the jmx reporter
+ */
+ public static void startJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().start();
+ }
+
+ /**
+ * Stops the jmx reporter
+ */
+ public static void stopJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().stop();
--- End diff --
Yep it stops the imx reporter. (there is only always one)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934712
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onConfigure
+ *
+ * @param joinPoint
+ * the join point (component) which is configured
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onConfigure
+ */
+ @Around("execution(* org.apache.wicket.Component.onConfigure(..))")
+ public Object aroundOnConfigure(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/configure", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onInitialize
+ *
+ * @param joinPoint
+ * the join point (component) which is initialized
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onInitialize
+ */
+ @Around("execution(* org.apache.wicket.Component.onInitialize(..))")
+ public Object aroundOnInitialize(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/initialize", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onDetach
+ *
+ * @param joinPoint
+ * the join point (component) which is calling detach
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while invoking onDetach
+ */
+ @Around("execution(* org.apache.wicket.Component.onDetach(..))")
+ public Object arroundOnDetach(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return mark("core/component/detach", joinPoint);
--- End diff --
maybe someone called this method manually and then this is a hint to resolve errors. I would let them in to have the data in comparison - wdyt?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217621566
You actually can't issue the time with IClassResolver how long the constructor takes to be processed for all components in that generic way - you also do not have any possibilities to issue all the metrics by using this functionality compared to AspectJ.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-195926576
I am going to create a ticket - soon! :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r56367067
--- Diff: wicket-experimental/wicket-atmosphere/pom.xml ---
@@ -24,6 +24,7 @@
<relativePath>../pom.xml</relativePath>
</parent>
<artifactId>wicket-atmosphere</artifactId>
+ <version>0.24-SNAPSHOT</version>
--- End diff --
:+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933817
--- Diff: wicket-metrics/src/main/resources/META-INF/aop.xml ---
@@ -0,0 +1,13 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+<aspectj>
+ <weaver options="-nowarn">
+ <include within="org.apache.wicket..*"/>
+ </weaver>
+ <aspects>
+ <aspect name="org.apache.wicket.metrics.aspects.BehaviorAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.IPartialPageRequestHandlerAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.ComponentAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.WicketFilterAspect" />
--- End diff --
If I want to add my custom aspect what should I do ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r56366866
--- Diff: wicket-experimental/wicket-metrics/src/main/resources/wicket-metrics.template.xml ---
@@ -0,0 +1,26 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+<aspectj>
+ <weaver options="-nowarn">
+ <include within="org.apache.wicket..*"/>
+ <include within="com.fluentsqaure..*"/>
--- End diff --
this is not needed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933847
--- Diff: wicket-user-guide/src/docs/guide/monitoring/monitoring_3.gdoc ---
@@ -0,0 +1,18 @@
+The data which is going to be measured depends on the wicket-metrics implementation. So it doesn't make any sense to collect time data
+
+about setRepsonsePage, but it does for the constructor of components, to see if a component needs a long time to be created. You can
--- End diff --
Typo in setRepsonsePage
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934679
--- Diff: wicket-metrics/src/main/resources/META-INF/aop.xml ---
@@ -0,0 +1,13 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+<aspectj>
+ <weaver options="-nowarn">
+ <include within="org.apache.wicket..*"/>
+ </weaver>
+ <aspects>
+ <aspect name="org.apache.wicket.metrics.aspects.BehaviorAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.IPartialPageRequestHandlerAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.ComponentAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.WicketFilterAspect" />
--- End diff --
you put in an aop.xml to your own META-INF folder in your class path and add the aspects there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933677
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ getMetricRegistry().meter(PREFIX + name + renderClassName(joinPoint)).mark();
+ }
+ if (joinPoint != null)
+ {
+ return joinPoint.proceed();
+ }
+ return null;
+ }
+
+ /**
+ * Stops the contex quietly
+ *
+ * @param context
+ * the context to stop
+ */
+ public void stopQuietly(Context context)
+ {
+ if (context != null)
+ {
+ context.stop();
+ }
+ }
+
+ /**
+ * Starts the jmx reporter
+ */
+ public static void startJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().start();
+ }
+
+ /**
+ * Stops the jmx reporter
+ */
+ public static void stopJmxReporter()
+ {
+ JmxReporter.forRegistry(getMetricRegistry()).build().stop();
+ }
+
+ /**
+ * If the metrics should be enabled
+ *
+ * @param enabled
+ * if the metrics should be enabled
+ */
+ public static void setEnabled(boolean enabled)
+ {
+ WicketMetrics.enabled = enabled;
--- End diff --
IMO this is not needed.
Better introduce IMetricsSettings + WicketMetrics.configure(application, metricsSettings).
See wicket-cdi, wicket-webjars, wicket-bootstrap for the pattern.
The settings could be used to configure which joinpoints to be enabled. E.g. some application may not be interested in how many behaviors have been instantiated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217770684
Hi - it is not only monitoring=true - please have a look at the documentation - you can configure every metrics by itself.
If we consider to replace the technology it should handle every part of the old tech + also give advantage.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55943224
--- Diff: wicket-metrics/src/main/resources/META-INF/aop.xml ---
@@ -0,0 +1,13 @@
+<!DOCTYPE aspectj PUBLIC "-//AspectJ//DTD//EN" "http://www.eclipse.org/aspectj/dtd/aspectj.dtd">
+<aspectj>
+ <weaver options="-nowarn">
+ <include within="org.apache.wicket..*"/>
+ </weaver>
+ <aspects>
+ <aspect name="org.apache.wicket.metrics.aspects.BehaviorAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.IPartialPageRequestHandlerAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.ComponentAspect" />
+ <aspect name="org.apache.wicket.metrics.aspects.WicketFilterAspect" />
--- End diff --
Yep I updated the user guide and described what is required to add metrics.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933619
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
--- End diff --
If there is MetricsRegistry in the metadata then it is enabled, otherwise - not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933812
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/WicketFilterAspect.java ---
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+
+/**
+ * Aspect to handle basic web application information
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class WicketFilterAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often a request has been made against the webapp and counts the time how
+ * long the request remains
+ *
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return returns the boolean of the processRequest method
+ *
+ * @throws Throwable
+ * might occur while invoking process request
+ */
+ @Around("execution(* org.apache.wicket.protocol.http.WicketFilter.processRequest(..))")
+ public Object aroundRequestProcessed(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/application/request", joinPoint);
--- End diff --
Here it will be more useful to have the request url (without the querystring) in the metrics data.
And a meter for each url.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217743743
1. It's subject to what instrument and what shouldn't. Right now all Page methods are being instrumented. If needed configuration might be stored somewhere: for example in MonitoringSettings and IClassResolver will use it to instrument just needed methods.
2. The most tricky point here: how instumented classes interact with the same class but not instrumented. I think if they devided by user sessions (for example only if monitoring=true set for me - only pages for my session should be instrumented) interaction is almost absent.
3. Sure. In other words: javassist is quite low level (but not so low as gclib or asm:)). So it depends only on you how to represent configuration in most useful way. I think we might have some XML file with definition of what classes should be instrumented, methods and what to measure. Then these metrics might be enabled or disabled in some MonitoringSettings
4. See above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933788
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/IPartialPageRequestHandlerAspect.java ---
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Aspect which checks ajax request targets
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+@Aspect
+public class IPartialPageRequestHandlerAspect extends WicketMetrics
--- End diff --
Any use cases for this aspect ?
When the number of calls to `#add()` might be interesting to me as a user?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933768
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onConfigure
+ *
+ * @param joinPoint
+ * the join point (component) which is configured
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onConfigure
+ */
+ @Around("execution(* org.apache.wicket.Component.onConfigure(..))")
+ public Object aroundOnConfigure(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/configure", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onInitialize
+ *
+ * @param joinPoint
+ * the join point (component) which is initialized
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onInitialize
+ */
+ @Around("execution(* org.apache.wicket.Component.onInitialize(..))")
+ public Object aroundOnInitialize(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/initialize", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onDetach
+ *
+ * @param joinPoint
+ * the join point (component) which is calling detach
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while invoking onDetach
+ */
+ @Around("execution(* org.apache.wicket.Component.onDetach(..))")
+ public Object arroundOnDetach(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return mark("core/component/detach", joinPoint);
--- End diff --
`#onDetach()` is called at least once for each rendered component by Wicket.
There is a ticket that it might be called even twice per request cycle.
I think this metric is not useful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934700
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/WicketFilterAspect.java ---
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+
+/**
+ * Aspect to handle basic web application information
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class WicketFilterAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often a request has been made against the webapp and counts the time how
+ * long the request remains
+ *
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return returns the boolean of the processRequest method
+ *
+ * @throws Throwable
+ * might occur while invoking process request
+ */
+ @Around("execution(* org.apache.wicket.protocol.http.WicketFilter.processRequest(..))")
+ public Object aroundRequestProcessed(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/application/request", joinPoint);
--- End diff --
I can get the information about the waypoint here - I can read out the url of the request past into the filter. I will do this change soon
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934719
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
--- End diff --
No problem I will change it to wicket's meta data.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217772405
Yes - that's also possible. Regarding changing of a technology: that was NOT my main point. There are just few additional requirements which will be good to cover and I just see/know how to do that with the aid of javassist. If you can do that with AspectJ: it would be also great.
Requirements:
1) No Java agent required for gathering metrics
2) Metrics can be enabled:
a) for everyone
b) just for designated user's sessions
c) for all pages and component
d) for selective set of component
3) Bouncing of a server is not required after configuration change
4) Metrics reporting might be plugable
Yea - when I will have more time - will try to replace AspectJ. (As I said before, I think that everything which AspectJ have - can be covered by Javassist - just because last one is more low level and both have the same inner nature.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55943215
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onConfigure
+ *
+ * @param joinPoint
+ * the join point (component) which is configured
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onConfigure
+ */
+ @Around("execution(* org.apache.wicket.Component.onConfigure(..))")
+ public Object aroundOnConfigure(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/configure", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onInitialize
+ *
+ * @param joinPoint
+ * the join point (component) which is initialized
+ * @return the object returned from the join point
+ *
+ * @throws Throwable
+ * might occur while invoking onInitialize
+ */
+ @Around("execution(* org.apache.wicket.Component.onInitialize(..))")
+ public Object aroundOnInitialize(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/initialize", joinPoint);
+ }
+
+ /**
+ * Collects data how often components calls onDetach
+ *
+ * @param joinPoint
+ * the join point (component) which is calling detach
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while invoking onDetach
+ */
+ @Around("execution(* org.apache.wicket.Component.onDetach(..))")
+ public Object arroundOnDetach(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return mark("core/component/detach", joinPoint);
--- End diff --
if you compare it to the render method. If there is a hugh gab, there is something wrong in the applications code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217679092
I prepared some PoC. Please find it here: https://github.com/PhantomYdn/wicket-bugs/tree/master/javassistpoc
Class https://github.com/PhantomYdn/wicket-bugs/blob/master/javassistpoc/src/main/java/ru/ydn/wicket/javassist/JAClassResolver.java does all the logic. And btw >50% of it is a copy paste of DefaultClassResolver (unfortunately needed methods marked final).
1) Run PoC by `mvn clean jetty:run`
2) Open http://localhost:8080 in a browser
3) Checkout system out
4) You will see something like this but much longer;)
```
ru.ydn.wicket.javassist.HomePage.renderPage() call took 35766587ns
ru.ydn.wicket.javassist.HomePage.isBookmarkable() call took 15989ns
ru.ydn.wicket.javassist.HomePage.getPageParameters() call took 7471ns
ru.ydn.wicket.javassist.HomePage.getPageParameters() call took 4880ns
ru.ydn.wicket.javassist.HomePage.onDetach() call took 23818ns
ru.ydn.wicket.javassist.HomePage.detachModel() call took 4562ns
ru.ydn.wicket.javassist.HomePage.detachModels() call took 48334ns
ru.ydn.wicket.javassist.HomePage.iterator() call took 5477ns
ru.ydn.wicket.javassist.HomePage.isBookmarkable() call took 8722ns
```
Space indentation is for highligting internal calls
Of course, it possible to instrument just selected methods for a profiler.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55943190
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
--- End diff --
Ah ok - I updated the user guide to show what to add to enable the measurements (with information about what is measured) - good idea!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934782
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
--- End diff --
done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217656619
@klopfdreh it has 3 licenses.
```
Javassist version 3.20.0-GA is available (June 25, 2015).
This version is distributed under the triple license of the MPL, the LGPL, and the Apache License. The distribution includes the jar file, the sources, some samples, the API manual, and a tutorial. The binary is also available from the central repo.
```
Source: http://jboss-javassist.github.io/javassist/
So there should no any problems.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by PhantomYdn <gi...@git.apache.org>.
Github user PhantomYdn commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217577593
Guys, question: why aspectj is required here? Wicket do allow instrument code through IClassResolver. Doing that through this class will allow do not load java agent and enable/disable this feature right at runtime.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55934790
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java ---
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics;
+
+import org.aspectj.lang.ProceedingJoinPoint;
+
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+/**
+ * Base aspect provides access to the metric registry
+ *
+ * @author Tobias Soloschenko
+ *
+ */
+public class WicketMetrics
+{
+
+ private static MetricRegistry metricRegistry;
+
+ private static boolean enabled = true;
+
+ private static final String PREFIX = "ApacheWicket/";
+
+ /**
+ * Gets the metric registry
+ *
+ * @return the metric registry
+ */
+ public static MetricRegistry getMetricRegistry()
+ {
+ if (metricRegistry == null)
+ {
+ metricRegistry = new MetricRegistry();
+ }
+ return metricRegistry;
+ }
+
+ /**
+ * Simply measure the time for a {@literal @}around
+ *
+ * @param name
+ * the name of the timer context
+ * @param joinPoint
+ * the joinPoint to be proceed
+ * @return the value of the join point
+ * @throws Throwable
+ * if there is an exception while execution
+ */
+ public Object measureTime(String name, ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ if (WicketMetrics.enabled)
+ {
+ Context context = getMetricRegistry().timer(PREFIX + name + renderClassName(joinPoint))
+ .time();
+ try
+ {
+ return joinPoint.proceed();
+ }
+ finally
+ {
+ stopQuietly(context);
+ }
+ }
+ else
+ {
+ return joinPoint.proceed();
+ }
+ }
+
+ /**
+ * Marks the meter with the given name
+ *
+ * @param name
+ * the name of the meter to be marked
+ * @param joinPoint
+ * the join point
+ * @return the result of the proceeded join point
+ * @throws Throwable
+ */
+ public Object mark(String name, ProceedingJoinPoint joinPoint) throws Throwable
--- End diff --
This only a wrapper to check if the metrics are enabled - and if so the metrics are collected but the functionality behind it is the same - mark / timer - so I thought to call it the same.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:
https://github.com/apache/wicket/pull/164#issuecomment-217771202
Oh and one other point:
If you have a class which extends Component:
Component > Panel > MyCustomPanel
Are you able to log information for it in this scenario.
In AspectJ you can read the joinPoint target class - what about javassist:
https://github.com/apache/wicket/blob/master/wicket-experimental/wicket-metrics/src/main/java/org/apache/wicket/metrics/WicketMetrics.java#L228
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933832
--- Diff: wicket-user-guide/src/docs/guide/monitoring/monitoring_1.gdoc ---
@@ -0,0 +1,19 @@
+This is a little example how to setup wicket-metrics within a tomcat.
--- End diff --
s/tomcat/Apache Tomcat/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: wicket-metrics
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/164#discussion_r55933753
--- Diff: wicket-metrics/src/main/java/org/apache/wicket/metrics/aspects/ComponentAspect.java ---
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.metrics.aspects;
+
+import org.apache.wicket.metrics.WicketMetrics;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * Gets information how often different components are rendered
+ *
+ * @author Tobias Soloschenko
+ */
+@Aspect
+public class ComponentAspect extends WicketMetrics
+{
+
+ /**
+ * Collects data how often components are rendered
+ *
+ * @param joinPoint
+ * the join point (component) which is rendered
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while onRender
+ */
+ @Around("execution(* org.apache.wicket.Component.onRender(..))")
+ public Object aroundOnRender(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/render", joinPoint);
+ }
+
+ /**
+ * Collects data how often components are created
+ *
+ * @param joinPoint
+ * the join point (component) which is created
+ * @return the object returned from the join point
+ * @throws Throwable
+ * might occur while constructing a new component
+ */
+ @Around("execution(org.apache.wicket.Component.new(..))")
+ public Object aroundNew(ProceedingJoinPoint joinPoint) throws Throwable
+ {
+ return measureTime("core/component/create", joinPoint);
--- End diff --
A useful setting here to disable the measurement for Wicket base classes.
This time would be useful for us to optimize where possible, but I guess most of the time user applications will be interested only in the time for their components.
Using regex filter would be the most flexible way but it might slow down the application.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---