Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

REVIEW: Add yammer metrics ( http://metrics.codahale.com ) to NX #779

Merged
merged 16 commits into from

4 participants

@adreghiciu
Owner

+1. IF we go further with this we should protect the exposed servlets via a filter chain as "noSessionCreation,authcBasic,perms[some nexus permission]"

@kellyrob99
Owner

+1 Do we have any concrete ideas about what metrics/healthchecks might be beneficial to track?

@jdillon
Owner

depends on for security:

#800

@adreghiciu
Owner

+1

@cstamas cstamas commented on the diff
...pe/nexus/apachehttpclient/InstrumentedHttpClient.java
@@ -0,0 +1,74 @@
+package org.sonatype.nexus.apachehttpclient;
@cstamas Owner
cstamas added a note

License is missing

@jdillon Owner
jdillon added a note

this is some duplicated/massaged code would need special handling of license here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cstamas cstamas commented on the diff
...xus/apachehttpclient/InstrumentedRequestDirector.java
@@ -0,0 +1,116 @@
+package org.sonatype.nexus.apachehttpclient;
@cstamas Owner
cstamas added a note

License is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cstamas cstamas commented on the diff
...-oss-webapp/src/main/resources/content/conf/jetty.xml
@@ -24,13 +24,17 @@
<!-- ============================================================ -->
<Configure id="Server" class="org.eclipse.jetty.server.Server">
<Set name="threadPool">
- <New class="org.sonatype.sisu.jetty.thread.InstrumentedQueuedThreadPool"/>
+ <New class="com.yammer.metrics.jetty.InstrumentedQueuedThreadPool"/>
@cstamas Owner
cstamas added a note

Just FTR: With this change you are removing this QTP
sonatype/sisu-jetty8@0cf9572

Some context about it here:
http://dev.eclipse.org/mhonarc/lists/jetty-users/msg02350.html

And (unrelated) issue describing Greg's stance wrt OOM here:
http://jira.codehaus.org/browse/JETTY-1194

Short summary: Jetty is silent about thread death, it does not inform you in any way that a thread was removed from a pool. While this problem with P2 was a special case (one single point acquired huge amounts of heap, that is immediately put out of scope, the P2 repo metadata DOM). But the point is, that the fact that QTP thread doing working on P2 related request was not logged anywhere, there was not signs at all, simply that one single request failed, and Nexus happily continued running, everything was seemingly working just fine.

While I do share Greg's concerns about OOM, the problem is that there is no feedback at all about OOM happened. And due to the nature of it, Nexus just continued happily working.

@jdillon Owner
jdillon added a note

@cstamas thanks for pointing this out. Is this still relevant, or does Jetty provide proper logging now? If you think org.sonatype.sisu.jetty.thread.InstrumentedQueuedThreadPool should be used instead, which is fine, then we can make a custom impl which captures metrics and does this strange thread wizardry.

BTW org.sonatype.sisu.jetty.thread.InstrumentedQueuedThreadPool ASIS does not really instrument anything, it just appears to use a thread factory and install a background thread... yes?


Also, some of the stock metrics integration classes have some minor issues which make them hard to re-use (like the httpclient bits). We may want to fix these upstream and not need to hack around them here.

@cstamas Owner
cstamas added a note

@jdillon No, Jetty does not provide anything in this area yet.

To repeat, the problem is that there is no feedback (in logs, visible) that OOM happened. This QTP modification was just a "best effort" try to make Nexus (well, Jetty) start nagging you if it detected "thread loss" from pool. Simply, you (the ops running Nexus) can be completely unaware of the fact that there was an OOM in system.

After reading a bit more, am leaning towards Greg's stance, I am fine with your change (removing this customized QTP and replacing it with Yammer one), and we should just recommend putting -XX:+HeapDumpOnOutOfMemoryError into wrapper.conf... as that's the best you could do so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cstamas
Owner

+1

@jdillon jdillon commented on the diff
...pe/nexus/apachehttpclient/InstrumentedHttpClient.java
((16 lines not shown))
+import com.yammer.metrics.core.MetricsRegistry;
+import com.yammer.metrics.httpclient.InstrumentedClientConnManager;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.http.ConnectionReuseStrategy;
+import org.apache.http.client.*;
+import org.apache.http.conn.ClientConnectionManager;
+import org.apache.http.conn.ConnectionKeepAliveStrategy;
+import org.apache.http.conn.routing.HttpRoutePlanner;
+import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.params.HttpParams;
+import org.apache.http.protocol.HttpProcessor;
+import org.apache.http.protocol.HttpRequestExecutor;
+
+// NOTE: Duplicated and augmented from original 2.2.0 source to change signature of CTOR ClientConnectionManager parameter
+// NOTE: Should get this changes into metrics-httpclient and avoid needing this
@jdillon Owner
jdillon added a note

Change to remove the need for this is currently up here:

jdillon/metrics@d59bbe0

Pending any other changes to simplify our integration before I make a pull request and lobby to get this change in. No clue how difficult that might be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jdillon jdillon merged commit b9e35a9 into master
@jdillon jdillon deleted the yammer-metrics branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 489 additions and 173 deletions.
  1. +18 −2 nexus-core/pom.xml
  2. +1 −2  nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/Hc4ProviderBase.java
  3. +2 −1  nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/Hc4ProviderImpl.java
  4. +86 −0 nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/InstrumentedHttpClient.java
  5. +128 −0 nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/InstrumentedRequestDirector.java
  6. +3 −2 nexus-core/src/main/java/org/sonatype/nexus/guice/NexusModules.java
  7. +2 −0  nexus-core/src/main/java/org/sonatype/nexus/plugins/DefaultNexusPluginManager.java
  8. +0 −39 nexus-core/src/main/java/org/sonatype/nexus/timing/Timed.java
  9. +0 −104 nexus-core/src/main/java/org/sonatype/nexus/timing/TimedInterceptor.java
  10. +26 −0 nexus-core/src/main/resources/META-INF/nexus/static-security.xml
  11. +4 −0 nexus-logging-extras/src/main/resources/META-INF/log/logback-nexus.xml
  12. +15 −0 nexus-oss-webapp/pom.xml
  13. +4 −0 nexus-oss-webapp/src/main/assembly/bundle.xml
  14. +4 −3 nexus-oss-webapp/src/main/resources/content/conf/jetty.xml
  15. +3 −1 ...ess-its/src/test/java/org/sonatype/nexus/integrationtests/upgrades/nexus652/Nexus652Beta5To10UpgradeIT.java
  16. +4 −1 nexus-test/nexus-test-harness-launcher/src/main/java/org/sonatype/nexus/test/booter/Jetty8NexusBooter.java
  17. +10 −0 nexus-webapp/pom.xml
  18. +122 −0 nexus-webapp/src/main/java/org/sonatype/nexus/webapp/MetricsModule.java
  19. +8 −12 ...exus/timing/TimingModule.java → nexus-webapp/src/main/java/org/sonatype/nexus/webapp/WebappModule.java
  20. +1 −1  ...ins/restlet1x/nexus-restlet1x-plugin/src/main/java/org/sonatype/nexus/rest/status/StatusPlexusResource.java
  21. +1 −1  plugins/siesta/nexus-siesta-test-plugin/src/main/java/org/sonatype/nexus/plugins/siesta/test/TestResource.java
  22. +47 −4 pom.xml
View
20 nexus-core/pom.xml
@@ -253,8 +253,24 @@
</dependency>
<dependency>
- <groupId>org.javasimon</groupId>
- <artifactId>javasimon-core</artifactId>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-core</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-guice</artifactId>
+ <exclusions>
+ <exclusion>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-httpclient</artifactId>
</dependency>
<!-- Testing -->
View
3  nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/Hc4ProviderBase.java
@@ -255,9 +255,8 @@ protected void configureProxy( final DefaultHttpClient httpClient, final RemoteP
* Sub-classed here to customize the http processor and to keep a sane logger name.
*/
private static class DefaultHttpClientImpl
- extends DefaultHttpClient
+ extends InstrumentedHttpClient
{
-
private DefaultHttpClientImpl( final ClientConnectionManager conman, final HttpParams params )
{
super( conman, params );
View
3  nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/Hc4ProviderImpl.java
@@ -18,6 +18,7 @@
import javax.inject.Named;
import javax.inject.Singleton;
+import com.yammer.metrics.httpclient.InstrumentedClientConnManager;
import org.apache.http.client.params.ClientPNames;
import org.apache.http.conn.ClientConnectionOperator;
import org.apache.http.conn.scheme.PlainSocketFactory;
@@ -269,7 +270,7 @@ protected PoolingClientConnectionManager createClientConnectionManager(
schemeRegistry.register( new Scheme( "http", 80, PlainSocketFactory.getSocketFactory() ) );
schemeRegistry.register( new Scheme( "https", 443, SSLSocketFactory.getSocketFactory() ) );
- final PoolingClientConnectionManager connManager = new PoolingClientConnectionManager( schemeRegistry )
+ final PoolingClientConnectionManager connManager = new InstrumentedClientConnManager( schemeRegistry )
{
@Override
protected ClientConnectionOperator createConnectionOperator( final SchemeRegistry defaultSchemeRegistry )
View
86 nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/InstrumentedHttpClient.java
@@ -0,0 +1,86 @@
+/*
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.apachehttpclient;
@cstamas Owner
cstamas added a note

License is missing

@jdillon Owner
jdillon added a note

this is some duplicated/massaged code would need special handling of license here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+import com.yammer.metrics.Metrics;
+import com.yammer.metrics.core.MetricsRegistry;
+import com.yammer.metrics.httpclient.InstrumentedClientConnManager;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.http.ConnectionReuseStrategy;
+import org.apache.http.client.*;
+import org.apache.http.conn.ClientConnectionManager;
+import org.apache.http.conn.ConnectionKeepAliveStrategy;
+import org.apache.http.conn.routing.HttpRoutePlanner;
+import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.params.HttpParams;
+import org.apache.http.protocol.HttpProcessor;
+import org.apache.http.protocol.HttpRequestExecutor;
+
+// NOTE: Duplicated and augmented from original 2.2.0 source to change signature of CTOR ClientConnectionManager parameter
+// NOTE: Should get this changes into metrics-httpclient and avoid needing this
@jdillon Owner
jdillon added a note

Change to remove the need for this is currently up here:

jdillon/metrics@d59bbe0

Pending any other changes to simplify our integration before I make a pull request and lobby to get this change in. No clue how difficult that might be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+public class InstrumentedHttpClient extends DefaultHttpClient {
+ private final Log log = LogFactory.getLog(getClass());
+
+ private final MetricsRegistry registry;
+
+ public InstrumentedHttpClient(MetricsRegistry registry,
+ ClientConnectionManager manager,
+ HttpParams params) {
+ super(manager, params);
+ this.registry = registry;
+ }
+
+ public InstrumentedHttpClient(ClientConnectionManager manager, HttpParams params) {
+ this(Metrics.defaultRegistry(), manager, params);
+ }
+
+ public InstrumentedHttpClient(HttpParams params) {
+ this(new InstrumentedClientConnManager(), params);
+ }
+
+ public InstrumentedHttpClient() {
+ this(null);
+ }
+
+ @Override
+ protected RequestDirector createClientRequestDirector(HttpRequestExecutor requestExec,
+ ClientConnectionManager conman,
+ ConnectionReuseStrategy reustrat,
+ ConnectionKeepAliveStrategy kastrat,
+ HttpRoutePlanner rouplan,
+ HttpProcessor httpProcessor,
+ HttpRequestRetryHandler retryHandler,
+ RedirectStrategy redirectStrategy,
+ AuthenticationStrategy targetAuthStrategy,
+ AuthenticationStrategy proxyAuthStrategy,
+ UserTokenHandler userTokenHandler,
+ HttpParams params) {
+ return new InstrumentedRequestDirector(
+ registry,
+ log,
+ requestExec,
+ conman,
+ reustrat,
+ kastrat,
+ rouplan,
+ httpProcessor,
+ retryHandler,
+ redirectStrategy,
+ targetAuthStrategy,
+ proxyAuthStrategy,
+ userTokenHandler,
+ params);
+ }
+}
View
128 nexus-core/src/main/java/org/sonatype/nexus/apachehttpclient/InstrumentedRequestDirector.java
@@ -0,0 +1,128 @@
+/*
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.apachehttpclient;
@cstamas Owner
cstamas added a note

License is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+import com.yammer.metrics.core.MetricsRegistry;
+import com.yammer.metrics.core.Timer;
+import com.yammer.metrics.core.TimerContext;
+import org.apache.commons.logging.Log;
+import org.apache.http.*;
+import org.apache.http.client.*;
+import org.apache.http.conn.ClientConnectionManager;
+import org.apache.http.conn.ConnectionKeepAliveStrategy;
+import org.apache.http.conn.routing.HttpRoutePlanner;
+import org.apache.http.impl.client.DefaultRequestDirector;
+import org.apache.http.params.HttpParams;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpProcessor;
+import org.apache.http.protocol.HttpRequestExecutor;
+
+import java.io.IOException;
+
+// NOTE: Duplicated and unchanged from original 2.2.0 source to support change in InstrumentedHttpClient
+// NOTE: Should get this changes into metrics-httpclient and avoid needing this
+
+class InstrumentedRequestDirector
+ extends DefaultRequestDirector {
+ private final static String GET = "GET", POST = "POST", HEAD = "HEAD", PUT = "PUT",
+ OPTIONS = "OPTIONS", DELETE = "DELETE", TRACE = "TRACE",
+ CONNECT = "CONNECT", MOVE = "MOVE", PATCH = "PATCH";
+
+ private final Timer getTimer;
+ private final Timer postTimer;
+ private final Timer headTimer;
+ private final Timer putTimer;
+ private final Timer deleteTimer;
+ private final Timer optionsTimer;
+ private final Timer traceTimer;
+ private final Timer connectTimer;
+ private final Timer moveTimer;
+ private final Timer patchTimer;
+ private final Timer otherTimer;
+
+ InstrumentedRequestDirector(MetricsRegistry registry,
+ Log log,
+ HttpRequestExecutor requestExec,
+ ClientConnectionManager conman,
+ ConnectionReuseStrategy reustrat,
+ ConnectionKeepAliveStrategy kastrat,
+ HttpRoutePlanner rouplan,
+ HttpProcessor httpProcessor,
+ HttpRequestRetryHandler retryHandler,
+ RedirectStrategy redirectStrategy,
+ AuthenticationStrategy targetAuthStrategy,
+ AuthenticationStrategy proxyAuthStrategy,
+ UserTokenHandler userTokenHandler,
+ HttpParams params) {
+ super(log,
+ requestExec,
+ conman,
+ reustrat,
+ kastrat,
+ rouplan,
+ httpProcessor,
+ retryHandler,
+ redirectStrategy,
+ targetAuthStrategy,
+ proxyAuthStrategy,
+ userTokenHandler,
+ params);
+ getTimer = registry.newTimer(HttpClient.class, "get-requests");
+ postTimer = registry.newTimer(HttpClient.class, "post-requests");
+ headTimer = registry.newTimer(HttpClient.class, "head-requests");
+ putTimer = registry.newTimer(HttpClient.class, "put-requests");
+ deleteTimer = registry.newTimer(HttpClient.class, "delete-requests");
+ optionsTimer = registry.newTimer(HttpClient.class, "options-requests");
+ traceTimer = registry.newTimer(HttpClient.class, "trace-requests");
+ connectTimer = registry.newTimer(HttpClient.class, "connect-requests");
+ moveTimer = registry.newTimer(HttpClient.class, "move-requests");
+ patchTimer = registry.newTimer(HttpClient.class, "patch-requests");
+ otherTimer = registry.newTimer(HttpClient.class, "other-requests");
+ }
+
+ @Override
+ public HttpResponse execute(HttpHost target, HttpRequest request, HttpContext context) throws HttpException, IOException {
+ final TimerContext timerContext = timer(request).time();
+ try {
+ return super.execute(target, request, context);
+ } finally {
+ timerContext.stop();
+ }
+ }
+
+ private Timer timer(HttpRequest request) {
+ final String method = request.getRequestLine().getMethod();
+ if (GET.equalsIgnoreCase(method)) {
+ return getTimer;
+ } else if (POST.equalsIgnoreCase(method)) {
+ return postTimer;
+ } else if (PUT.equalsIgnoreCase(method)) {
+ return putTimer;
+ } else if (HEAD.equalsIgnoreCase(method)) {
+ return headTimer;
+ } else if (DELETE.equalsIgnoreCase(method)) {
+ return deleteTimer;
+ } else if (OPTIONS.equalsIgnoreCase(method)) {
+ return optionsTimer;
+ } else if (TRACE.equalsIgnoreCase(method)) {
+ return traceTimer;
+ } else if (CONNECT.equalsIgnoreCase(method)) {
+ return connectTimer;
+ } else if (PATCH.equalsIgnoreCase(method)) {
+ return patchTimer;
+ } else if (MOVE.equalsIgnoreCase(method)) {
+ return moveTimer;
+ }
+ return otherTimer;
+ }
+}
View
5 nexus-core/src/main/java/org/sonatype/nexus/guice/NexusModules.java
@@ -13,9 +13,9 @@
package org.sonatype.nexus.guice;
-import org.apache.shiro.guice.aop.ShiroAopModule;
-
import com.google.inject.AbstractModule;
+import com.yammer.metrics.guice.InstrumentationModule;
+import org.apache.shiro.guice.aop.ShiroAopModule;
/**
* Nexus guice modules.
@@ -33,6 +33,7 @@
@Override
protected void configure() {
install(new ShiroAopModule());
+ install(new InstrumentationModule());
}
}
View
2  nexus-core/src/main/java/org/sonatype/nexus/plugins/DefaultNexusPluginManager.java
@@ -30,6 +30,7 @@
import javax.inject.Named;
import javax.inject.Singleton;
+import com.yammer.metrics.annotation.Timed;
import org.codehaus.plexus.DefaultPlexusContainer;
import org.codehaus.plexus.classworlds.realm.ClassRealm;
import org.codehaus.plexus.classworlds.realm.DuplicateRealmException;
@@ -137,6 +138,7 @@ public DefaultNexusPluginManager( final RepositoryTypeRegistry repositoryTypeReg
return new HashMap<GAVCoordinate, PluginResponse>( pluginResponses );
}
+ @Timed
public Collection<PluginManagerResponse> activateInstalledPlugins()
{
final List<PluginManagerResponse> result = new ArrayList<PluginManagerResponse>();
View
39 nexus-core/src/main/java/org/sonatype/nexus/timing/Timed.java
@@ -1,39 +0,0 @@
-/*
- * Sonatype Nexus (TM) Open Source Version
- * Copyright (c) 2007-2012 Sonatype, Inc.
- * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
- *
- * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
- * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
- *
- * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
- * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
- * Eclipse Foundation. All other trademarks are the property of their respective owners.
- */
-package org.sonatype.nexus.timing;
-
-import javax.inject.Qualifier;
-import java.lang.annotation.Retention;
-import java.lang.annotation.Target;
-
-import static java.lang.annotation.ElementType.METHOD;
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-/**
- * Marker for methods which should have timing details sampled.
- *
- * @since 2.4
- */
-@Qualifier
-@Target({METHOD})
-@Retention(RUNTIME)
-public @interface Timed
-{
- String DEFAULT_VALUE = "";
-
- /**
- * The name of the timing metric. If left as {@link #DEFAULT_VALUE} or a blank/empty string,
- * then a name will be determined from the aspect context around the method being woven.
- */
- String value() default DEFAULT_VALUE;
-}
View
104 nexus-core/src/main/java/org/sonatype/nexus/timing/TimedInterceptor.java
@@ -1,104 +0,0 @@
-/*
- * Sonatype Nexus (TM) Open Source Version
- * Copyright (c) 2007-2012 Sonatype, Inc.
- * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
- *
- * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
- * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
- *
- * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
- * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
- * Eclipse Foundation. All other trademarks are the property of their respective owners.
- */
-package org.sonatype.nexus.timing;
-
-import org.aopalliance.intercept.MethodInterceptor;
-import org.aopalliance.intercept.MethodInvocation;
-import org.javasimon.SimonManager;
-import org.javasimon.Split;
-import org.javasimon.Stopwatch;
-import org.jsoup.helper.StringUtil;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.slf4j.Marker;
-import org.slf4j.MarkerFactory;
-
-import java.lang.reflect.Method;
-
-/**
- * Handles {@link Timed} method invocation timing.
- *
- * @since 2.4
- */
-public class TimedInterceptor
- implements MethodInterceptor
-{
- private static final Marker TIMING = MarkerFactory.getMarker("TIMING");
-
- private static final Logger log = LoggerFactory.getLogger(TimedInterceptor.class);
-
- // TODO: Sort out javasimon configuration, JMX and callbacks to handling logging or not, etc.
- // TODO: Probably want to do all of this ^^^ in a helper class
-
- // TODO: Could potentially implement a simple cache here if we think perf of name calculation is too high
-
- /**
- * Construct name from a {@link MethodInvocation}. If {@link Timed#value()} is default/blank, then
- * a name will be constructed from the given {@link Method}.
- *
- * @see #nameOf(Method)
- */
- private String nameOf(final MethodInvocation invocation) {
- Method method = invocation.getMethod();
- Timed annotation = method.getAnnotation(Timed.class);
- String name = Timed.DEFAULT_VALUE;
-
- // This should not happen, as we only intercept methods which have @Timed on them, but just in-case
- if (annotation == null) {
- log.error("Missing @Timed annotation on method: {}", method);
- // will construct an automatic name
- }
- else {
- name = annotation.value();
- }
-
- // autodetect a name if needed
- if (StringUtil.isBlank(name)) {
- name = nameOf(method);
- }
-
- return name;
- }
-
- /**
- * Construct name from a {@link Method}.
- */
- private String nameOf(final Method method) {
- return String.format("%s.%s-%s",
- method.getDeclaringClass().getName(),
- method.getName(),
- method.getParameterTypes().length);
- }
-
- /**
- * Wrap method invocation with a Javasimon {@link Stopwatch} to capture timing metrics.
- */
- @Override
- public Object invoke(final MethodInvocation invocation) throws Throwable {
- String name = nameOf(invocation);
- log.trace("Timing invocation: [{}] {}", name, invocation.getMethod());
-
- // TODO: Sort out how best to use javasimon here for core + plugins, etc.
- Stopwatch watch = SimonManager.getStopwatch(name);
- Split split = watch.start();
- try {
- return invocation.proceed();
- }
- finally {
- split.stop();
-
- // TODO: Remove logging here and handle by a callback or only expose via JMX?
- log.trace(TIMING, "{}", watch);
- }
- }
-}
View
26 nexus-core/src/main/resources/META-INF/nexus/static-security.xml
@@ -311,10 +311,36 @@
<privilege>83</privilege>
</privileges>
</role>
+ <role>
+ <id>metrics-endpoints</id>
+ <name>Metrics Endpoints</name>
+ <description>Allows access to metrics endpoints.</description>
+ <sessionTimeout>60</sessionTimeout>
+ <privileges>
+ <privilege>metrics-endpoints</privilege>
+ </privileges>
+ </role>
</roles>
<privileges>
<privilege>
+ <id>metrics-endpoints</id>
+ <type>method</type>
+ <name>Metrics Endpoints</name>
+ <description>Allows access to metrics endpoints.</description>
+ <properties>
+ <property>
+ <key>method</key>
+ <value>*</value>
+ </property>
+ <property>
+ <key>permission</key>
+ <value>nexus:metrics-endpoints</value>
+ </property>
+ </properties>
+ </privilege>
+
+ <privilege>
<id>T1</id>
<type>target</type>
<name>All M2 Repositories - (read)</name>
View
4 nexus-logging-extras/src/main/resources/META-INF/log/logback-nexus.xml
@@ -23,6 +23,7 @@
<pattern>${appender.pattern}</pattern>
</encoder>
</appender>
+
<appender name="logfile" class="ch.qos.logback.core.rolling.RollingFileAppender">
<File>${appender.file}</File>
<Append>true</Append>
@@ -34,6 +35,8 @@
</rollingPolicy>
</appender>
+ <appender name="metrics" class="com.yammer.metrics.logback.InstrumentedAppender"/>
+
<logger name="org.sonatype.nexus.rest.NexusApplication" level="WARN" />
<logger name="httpclient" level="INFO" />
<logger name="org.apache.http" level="INFO" />
@@ -47,6 +50,7 @@
<root level="${root.level}">
<appender-ref ref="console" />
<appender-ref ref="logfile" />
+ <appender-ref ref="metrics" />
</root>
</included>
View
15 nexus-oss-webapp/pom.xml
@@ -123,6 +123,21 @@
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-jmx</artifactId>
</dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-core</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-jetty</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-logback</artifactId>
+ </dependency>
</dependencies>
<build>
View
4 nexus-oss-webapp/src/main/assembly/bundle.xml
@@ -106,6 +106,7 @@
<exclude>WEB-INF/lib/logback-core*.jar</exclude>
<exclude>WEB-INF/lib/logback-classic*.jar</exclude>
<exclude>WEB-INF/lib/nexus-logging-extras-appender*.jar</exclude>
+ <exclude>WEB-INF/lib/metrics-core*.jar</exclude>
<exclude>WEB-INF/classes/logback.xml</exclude>
</excludes>
</unpackOptions>
@@ -128,6 +129,9 @@
<include>org.sonatype.appcontext:*</include>
<include>org.slf4j:*</include>
<include>ch.qos.logback:*</include>
+ <include>com.yammer.metrics:metrics-core</include>
+ <include>com.yammer.metrics:metrics-jetty</include>
+ <include>com.yammer.metrics:metrics-logback</include>
</includes>
</dependencySet>
</dependencySets>
View
7 nexus-oss-webapp/src/main/resources/content/conf/jetty.xml
@@ -24,13 +24,14 @@
<!-- ============================================================ -->
<Configure id="Server" class="org.eclipse.jetty.server.Server">
<Set name="threadPool">
- <New class="org.sonatype.sisu.jetty.thread.InstrumentedQueuedThreadPool"/>
+ <New class="com.yammer.metrics.jetty.InstrumentedQueuedThreadPool"/>
@cstamas Owner
cstamas added a note

Just FTR: With this change you are removing this QTP
sonatype/sisu-jetty8@0cf9572

Some context about it here:
http://dev.eclipse.org/mhonarc/lists/jetty-users/msg02350.html

And (unrelated) issue describing Greg's stance wrt OOM here:
http://jira.codehaus.org/browse/JETTY-1194

Short summary: Jetty is silent about thread death, it does not inform you in any way that a thread was removed from a pool. While this problem with P2 was a special case (one single point acquired huge amounts of heap, that is immediately put out of scope, the P2 repo metadata DOM). But the point is, that the fact that QTP thread doing working on P2 related request was not logged anywhere, there was not signs at all, simply that one single request failed, and Nexus happily continued running, everything was seemingly working just fine.

While I do share Greg's concerns about OOM, the problem is that there is no feedback at all about OOM happened. And due to the nature of it, Nexus just continued happily working.

@jdillon Owner
jdillon added a note

@cstamas thanks for pointing this out. Is this still relevant, or does Jetty provide proper logging now? If you think org.sonatype.sisu.jetty.thread.InstrumentedQueuedThreadPool should be used instead, which is fine, then we can make a custom impl which captures metrics and does this strange thread wizardry.

BTW org.sonatype.sisu.jetty.thread.InstrumentedQueuedThreadPool ASIS does not really instrument anything, it just appears to use a thread factory and install a background thread... yes?


Also, some of the stock metrics integration classes have some minor issues which make them hard to re-use (like the httpclient bits). We may want to fix these upstream and not need to hack around them here.

@cstamas Owner
cstamas added a note

@jdillon No, Jetty does not provide anything in this area yet.

To repeat, the problem is that there is no feedback (in logs, visible) that OOM happened. This QTP modification was just a "best effort" try to make Nexus (well, Jetty) start nagging you if it detected "thread loss" from pool. Simply, you (the ops running Nexus) can be completely unaware of the fact that there was an OOM in system.

After reading a bit more, am leaning towards Greg's stance, I am fine with your change (removing this customized QTP and replacing it with Yammer one), and we should just recommend putting -XX:+HeapDumpOnOutOfMemoryError into wrapper.conf... as that's the best you could do so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
</Set>
<Call name="addConnector">
<Arg>
- <New class="org.eclipse.jetty.server.nio.SelectChannelConnector">
+ <New class="com.yammer.metrics.jetty.InstrumentedSelectChannelConnector">
+ <!-- metrics integration needs port on ctor invocation -->
+ <Arg type="int">${application-port}</Arg>
<Set name="host">${application-host}</Set>
- <Set name="port">${application-port}</Set>
</New>
</Arg>
</Call>
View
4 ...-its/src/test/java/org/sonatype/nexus/integrationtests/upgrades/nexus652/Nexus652Beta5To10UpgradeIT.java
@@ -87,6 +87,8 @@ public void checkNexusConfig()
public void checkSecurityConfig()
throws IOException
{
+ // FIXME: This is a pretty shitty integration test... not sure this is worth the ~30 seconds it takes to run this
+
org.sonatype.security.model.Configuration secConfig = getSecurityConfigUtil().getSecurityConfig();
Assert.assertEquals( "User Count:", secConfig.getUsers().size(), 7 );
@@ -95,7 +97,7 @@ public void checkSecurityConfig()
{
roleIds.add( role.getId() );
}
- Assert.assertEquals( "Roles Count differs, expected: 29, found: " + roleIds, secConfig.getRoles().size(), 29 );
+ Assert.assertEquals( "Roles Count differs, expected: 30, found: " + roleIds, secConfig.getRoles().size(), 30 );
// again, everything should have been upgraded.
}
View
5 nexus-test/nexus-test-harness-launcher/src/main/java/org/sonatype/nexus/test/booter/Jetty8NexusBooter.java
@@ -450,7 +450,7 @@ protected void tamperJarsForSharedClasspath( final File basedir )
// tamperJarsForSharedClasspath( basedir, sharedLibs, "slf4j-*.jar" );
// tamperJarsForSharedClasspath( basedir, sharedLibs, "logback-*.jar" );
- // move jetty (actualy, all that is level up in real bundle too) level up, it is isolated anyway in real bundle
+ // move jetty (actually, all that is level up in real bundle too) level up, it is isolated anyway in real bundle
tamperJarsForSharedClasspath( basedir, sharedLibs, "jetty-*.jar" );
tamperJarsForSharedClasspath( basedir, sharedLibs, "javax.servlet-*.jar" );
tamperJarsForSharedClasspath( basedir, sharedLibs, "sisu-jetty8-*.jar" );
@@ -461,6 +461,9 @@ protected void tamperJarsForSharedClasspath( final File basedir )
tamperJarsForSharedClasspath( basedir, sharedLibs, "slf4j-*.jar" );
tamperJarsForSharedClasspath( basedir, sharedLibs, "logback-*.jar" );
tamperJarsForSharedClasspath( basedir, sharedLibs, "nexus-logging-extras-appender-*.jar" );
+ tamperJarsForSharedClasspath( basedir, sharedLibs, "metrics-core-*.jar" );
+ tamperJarsForSharedClasspath( basedir, sharedLibs, "metrics-jetty-*.jar" );
+ tamperJarsForSharedClasspath( basedir, sharedLibs, "metrics-logback-*.jar" );
}
/**
View
10 nexus-webapp/pom.xml
@@ -73,6 +73,16 @@
</dependency>
<dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-servlet</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-web</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>org.sonatype.nexus</groupId>
<artifactId>nexus-oss-edition</artifactId>
</dependency>
View
122 nexus-webapp/src/main/java/org/sonatype/nexus/webapp/MetricsModule.java
@@ -0,0 +1,122 @@
+/*
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.webapp;
+
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.inject.AbstractModule;
+import com.google.inject.servlet.ServletModule;
+import com.yammer.metrics.HealthChecks;
+import com.yammer.metrics.Metrics;
+import com.yammer.metrics.core.Clock;
+import com.yammer.metrics.core.HealthCheckRegistry;
+import com.yammer.metrics.core.MetricsRegistry;
+import com.yammer.metrics.core.VirtualMachineMetrics;
+import com.yammer.metrics.reporting.HealthCheckServlet;
+import com.yammer.metrics.reporting.MetricsServlet;
+import com.yammer.metrics.reporting.PingServlet;
+import com.yammer.metrics.reporting.ThreadDumpServlet;
+import com.yammer.metrics.util.DeadlockHealthCheck;
+import com.yammer.metrics.web.DefaultWebappMetricsFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.sonatype.nexus.guice.FilterChainModule;
+import org.sonatype.security.web.guice.SecurityWebFilter;
+
+/**
+ * <a href="http://metrics.codahale.com">Yammer Metrics</a> guice configuration.
+ *
+ * Installs servlet endpoints:
+ *
+ * <ul>
+ * <li>/internal/ping</li>
+ * <li>/internal/threads</li>
+ * <li>/internal/metrics</li>
+ * <li>/internal/healthcheck</li>
+ * </ul>
+ *
+ * Protected by {@code nexus:metrics-endpoints} permission.
+ *
+ * @since 2.5
+ */
+public class MetricsModule
+ extends AbstractModule
+{
+ private static final Logger log = LoggerFactory.getLogger(MetricsModule.class);
+
+ private static final String MOUNT_POINT = "/internal";
+
+ @Override
+ protected void configure() {
+ // NOTE: AdminServletModule (metrics-guice intgegration) generates invalid links, so wire up servlets ourselves
+
+ final Clock clock = Clock.defaultClock();
+ bind(Clock.class).toInstance(clock);
+
+ final VirtualMachineMetrics virtualMachineMetrics = VirtualMachineMetrics.getInstance();
+ bind(VirtualMachineMetrics.class).toInstance(virtualMachineMetrics);
+
+ final HealthCheckRegistry healthCheckRegistry = HealthChecks.defaultRegistry();
+ bind(HealthCheckRegistry.class).toInstance(healthCheckRegistry);
+
+ // TODO: Consider making a component which can mediate (via sisu) adding/removing healthcheck components to/from the registry
+ // TODO: For now new instances must be explicitly registered, like this one:
+ healthCheckRegistry.register(new DeadlockHealthCheck(virtualMachineMetrics));
+
+ final MetricsRegistry metricsRegistry = Metrics.defaultRegistry();
+ bind(MetricsRegistry.class).toInstance(metricsRegistry);
+
+ final JsonFactory jsonFactory = new JsonFactory(new ObjectMapper());
+ bind(JsonFactory.class).toInstance(jsonFactory);
+
+ // FIXME: Sort out if we need to do this. Its noted in SiestaModule, may be needed here too :-(
+ //bind(SecurityWebFilter.class);
+
+ install(new ServletModule()
+ {
+ @Override
+ protected void configureServlets() {
+ serve(MOUNT_POINT + "/ping").with(new PingServlet());
+
+ serve(MOUNT_POINT + "/threads").with(new ThreadDumpServlet(virtualMachineMetrics));
+
+ serve(MOUNT_POINT + "/metrics").with(new MetricsServlet(
+ clock,
+ virtualMachineMetrics,
+ metricsRegistry,
+ jsonFactory,
+ true
+ ));
+
+ serve(MOUNT_POINT + "/healthcheck").with(new HealthCheckServlet(healthCheckRegistry));
+
+ // record metrics for all webapp access
+ filter("/*").through(new DefaultWebappMetricsFilter());
+
+ // configure security
+ filter(MOUNT_POINT + "/*").through(SecurityWebFilter.class);
+ }
+ });
+
+ // require permission to use endpoints
+ install(new FilterChainModule()
+ {
+ @Override
+ protected void configure() {
+ addFilterChain(MOUNT_POINT + "/**", "noSessionCreation,authcBasic,perms[nexus:metrics-endpoints]");
+ }
+ });
+
+ log.info("Metrics support configured");
+ }
+}
View
20 .../java/org/sonatype/nexus/timing/TimingModule.java → .../java/org/sonatype/nexus/webapp/WebappModule.java
@@ -10,27 +10,23 @@
* of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
* Eclipse Foundation. All other trademarks are the property of their respective owners.
*/
-package org.sonatype.nexus.timing;
+package org.sonatype.nexus.webapp;
-import javax.inject.Named;
-
-import org.sonatype.nexus.guice.AbstractInterceptorModule;
+import com.google.inject.AbstractModule;
-import com.google.inject.matcher.Matchers;
+import javax.inject.Named;
/**
- * Adds support for {@link Timed} method invocations.
- *
- * @see TimedInterceptor
+ * Nexus webapp module.
*
- * @since 2.4
+ * @since 2.5
*/
@Named
-public class TimingModule
- extends AbstractInterceptorModule
+public class WebappModule
+ extends AbstractModule
{
@Override
protected void configure() {
- bindInterceptor(Matchers.any(), Matchers.annotatedWith(Timed.class), new TimedInterceptor());
+ install(new MetricsModule());
}
}
View
2  .../restlet1x/nexus-restlet1x-plugin/src/main/java/org/sonatype/nexus/rest/status/StatusPlexusResource.java
@@ -19,6 +19,7 @@
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
+import com.yammer.metrics.annotation.Timed;
import org.codehaus.enunciate.contract.jaxrs.ResourceMethodSignature;
import org.codehaus.plexus.component.annotations.Component;
import org.codehaus.plexus.component.annotations.Requirement;
@@ -32,7 +33,6 @@
import org.sonatype.nexus.rest.model.NexusAuthenticationClientPermissions;
import org.sonatype.nexus.rest.model.StatusResource;
import org.sonatype.nexus.rest.model.StatusResourceResponse;
-import org.sonatype.nexus.timing.Timed;
import org.sonatype.plexus.rest.resource.ManagedPlexusResource;
import org.sonatype.plexus.rest.resource.PathProtectionDescriptor;
import org.sonatype.security.rest.authentication.AbstractUIPermissionCalculatingPlexusResource;
View
2  ...s/siesta/nexus-siesta-test-plugin/src/main/java/org/sonatype/nexus/plugins/siesta/test/TestResource.java
@@ -12,11 +12,11 @@
*/
package org.sonatype.nexus.plugins.siesta.test;
+import com.yammer.metrics.annotation.Timed;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonatype.nexus.configuration.application.ApplicationConfiguration;
import org.sonatype.nexus.plugins.siesta.test.model.UserXO;
-import org.sonatype.nexus.timing.Timed;
import org.sonatype.sisu.siesta.common.Resource;
import org.apache.shiro.authz.annotation.RequiresPermissions;
View
51 pom.xml
@@ -176,10 +176,11 @@
<aether.version>1.13.1</aether.version>
<jetty.version>8.1.8.v20121106</jetty.version>
<maven.version>3.0.4</maven.version>
- <plexus-security.version>3.0.2</plexus-security.version>
+ <plexus-security.version>3.0.3-SNAPSHOT</plexus-security.version>
<shiro.version>1.2.1</shiro.version>
<jackson.version>1.9.10</jackson.version>
<goodies.version>1.6</goodies.version>
+ <metrics.version>2.2.0</metrics.version>
<!-- See 'idea' profile. -->
<nexus-plugin.type>nexus-plugin</nexus-plugin.type>
@@ -831,9 +832,51 @@
</dependency>
<dependency>
- <groupId>org.javasimon</groupId>
- <artifactId>javasimon-core</artifactId>
- <version>3.3.0</version>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-core</artifactId>
+ <version>${metrics.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-guice</artifactId>
+ <version>${metrics.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-servlet</artifactId>
+ <version>${metrics.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-jersey</artifactId>
+ <version>${metrics.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-jetty</artifactId>
+ <version>${metrics.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-httpclient</artifactId>
+ <version>${metrics.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-logback</artifactId>
+ <version>${metrics.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.yammer.metrics</groupId>
+ <artifactId>metrics-web</artifactId>
+ <version>${metrics.version}</version>
</dependency>
<!-- INTERNAL -->
Something went wrong with that request. Please try again.