From c53c4e4b023ed6004adc1668871272a3623d1f7e Mon Sep 17 00:00:00 2001 From: lytscu Date: Mon, 13 Nov 2017 16:44:04 +0800 Subject: [PATCH] Modify the code review --- .../v2/MongoDBV2MethodInterceptor.java | 35 ++++++-- .../MongoDBCollectionInstrumentation.java | 70 +++++++-------- .../v2/define/MongoDBV2Instrumentation.java | 89 +------------------ 3 files changed, 69 insertions(+), 125 deletions(-) diff --git a/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/MongoDBV2MethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/MongoDBV2MethodInterceptor.java index acbeea1e7..51542336e 100644 --- a/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/MongoDBV2MethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/MongoDBV2MethodInterceptor.java @@ -18,8 +18,12 @@ package org.skywalking.apm.plugin.mongodb.v2; +import com.mongodb.CommandResult; +import com.mongodb.Mongo; import com.mongodb.ServerAddress; import com.mongodb.DB; +import com.mongodb.WriteResult; +import com.mongodb.AggregationOutput; import java.lang.reflect.Method; import java.util.List; import org.skywalking.apm.agent.core.context.ContextCarrier; @@ -35,9 +39,7 @@ import org.skywalking.apm.network.trace.component.ComponentsDefine; /** * {@link MongoDBV2MethodInterceptor} intercept method of {@link com.mongodb.DBCollection#find()} - *or{@link com.mongodb.DBCollectionImpl#insertImpl}.... record the mongoDB host, operation name ... - * - * @Auther liyuntao + * or{@link com.mongodb.DBCollectionImpl#insertImpl}.... record the mongoDB host, operation name ... */ public class MongoDBV2MethodInterceptor implements InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor { @@ -59,6 +61,23 @@ public class MongoDBV2MethodInterceptor implements InstanceMethodsAroundIntercep @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { + AbstractSpan activeSpan = ContextManager.activeSpan(); + if (ret instanceof WriteResult) { + WriteResult wresult = (WriteResult)ret; + if (!wresult.getCachedLastError().ok()) { + Tags.STATUS_CODE.set(activeSpan, "failed"); + } + } else if (ret instanceof CommandResult) { + CommandResult cresult = (CommandResult)ret; + if (!cresult.ok()) { + Tags.STATUS_CODE.set(activeSpan, "failed"); + } + } else if (ret instanceof AggregationOutput) { + AggregationOutput aresult = (AggregationOutput)ret; + if (!aresult.getCommandResult().ok()) { + Tags.STATUS_CODE.set(activeSpan, "failed"); + } + } ContextManager.stopSpan(); return ret; } @@ -72,8 +91,14 @@ public class MongoDBV2MethodInterceptor implements InstanceMethodsAroundIntercep @Override public void onConstruct(EnhancedInstance objInst, Object[] allArguments) { - DB db = (DB)allArguments[0]; - List servers = db.getMongo().getAllAddress(); + List servers = null; + if (allArguments[0] instanceof DB) { + DB db = (DB)allArguments[0]; + servers = db.getMongo().getAllAddress(); + } else if (allArguments[0] instanceof Mongo) { + Mongo mongo = (Mongo)allArguments[0]; + servers = mongo.getAllAddress(); + } StringBuilder peers = new StringBuilder(); for (ServerAddress address : servers) { diff --git a/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBCollectionInstrumentation.java b/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBCollectionInstrumentation.java index 7e27b92bf..9ac7fd445 100644 --- a/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBCollectionInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBCollectionInstrumentation.java @@ -76,38 +76,6 @@ public class MongoDBCollectionInstrumentation extends ClassInstanceMethodsEnhanc return false; } }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("mapReduce").and(takesArgumentWithType(0, "com.mongodb.MapReduceCommand")); - } - - @Override - public String getMethodsInterceptor() { - return MONGDB_METHOD_INTERCET_CLASS; - } - - @Override - public boolean isOverrideArgs() { - return false; - } - }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("aggregate").and(takesArgumentWithType(1, "com.mongodb.ReadPreference")); - } - - @Override - public String getMethodsInterceptor() { - return MONGDB_METHOD_INTERCET_CLASS; - } - - @Override - public boolean isOverrideArgs() { - return false; - } - }, new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { @@ -127,7 +95,7 @@ public class MongoDBCollectionInstrumentation extends ClassInstanceMethodsEnhanc new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return named("createIndex").and(takesArgumentWithType(2, "boolean")); + return named("insert"); } @Override @@ -143,7 +111,7 @@ public class MongoDBCollectionInstrumentation extends ClassInstanceMethodsEnhanc new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return named("insert").and(takesArgumentWithType(2, "com.mongodb.DBEncoder")); + return named("update"); } @Override @@ -159,7 +127,39 @@ public class MongoDBCollectionInstrumentation extends ClassInstanceMethodsEnhanc new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return named("update").and(takesArgumentWithType(5, "com.mongodb.DBEncoder")); + return named("remove").and(takesArgumentWithType(2, "com.mongodb.DBEncoder")); + } + + @Override + public String getMethodsInterceptor() { + return MONGDB_METHOD_INTERCET_CLASS; + } + + @Override + public boolean isOverrideArgs() { + return false; + } + }, + new InstanceMethodsInterceptPoint() { + @Override + public ElementMatcher getMethodsMatcher() { + return named("findAndModify"); + } + + @Override + public String getMethodsInterceptor() { + return MONGDB_METHOD_INTERCET_CLASS; + } + + @Override + public boolean isOverrideArgs() { + return false; + } + }, + new InstanceMethodsInterceptPoint() { + @Override + public ElementMatcher getMethodsMatcher() { + return named("createIndex").and(takesArgumentWithType(2, "com.mongodb.DBEncoder")); } @Override diff --git a/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBV2Instrumentation.java b/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBV2Instrumentation.java index edddee003..039297552 100644 --- a/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBV2Instrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/mongodb-2.x-plugin/src/main/java/org/skywalking/apm/plugin/mongodb/v2/define/MongoDBV2Instrumentation.java @@ -31,13 +31,12 @@ import static org.skywalking.apm.agent.core.plugin.bytebuddy.ArgumentTypeNameMat import static org.skywalking.apm.agent.core.plugin.match.NameMatch.byName; /** - * {@link MongoDBCollectionInstrumentation} presents that skywalking intercepts {@link - * com.mongodb.DBCollectionImpl#find()}, {@link com.mongodb.DBCollectionImpl#insertImpl} by using {@link - * MongoDBV2MethodInterceptor}. + * {@link MongoDBV2Instrumentation} presents that skywalking intercepts {@link com.mongodb.DB#command}, + * {@link com.mongodb.DBCollection#mapReduce} by using {@link MongoDBV2MethodInterceptor}. */ public class MongoDBV2Instrumentation extends ClassInstanceMethodsEnhancePluginDefine { - private static final String ENHANCE_CLASS = "com.mongodb.DBCollectionImpl"; + private static final String ENHANCE_CLASS = "com.mongodb.DB"; private static final String MONGDB_METHOD_INTERCET_CLASS = "org.skywalking.apm.plugin.mongodb.v2.MongoDBV2MethodInterceptor"; @@ -65,7 +64,7 @@ public class MongoDBV2Instrumentation extends ClassInstanceMethodsEnhancePluginD new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return named("insertImpl"); + return named("command").and(takesArgumentWithType(3, "com.mongodb.DBEncoder")); } @Override @@ -78,86 +77,6 @@ public class MongoDBV2Instrumentation extends ClassInstanceMethodsEnhancePluginD return false; } }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("remove").and(takesArgumentWithType(1, "boolean")); - } - - @Override - public String getMethodsInterceptor() { - return MONGDB_METHOD_INTERCET_CLASS; - } - - @Override - public boolean isOverrideArgs() { - return false; - } - }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("updateImpl"); - } - - @Override - public String getMethodsInterceptor() { - return MONGDB_METHOD_INTERCET_CLASS; - } - - @Override - public boolean isOverrideArgs() { - return false; - } - }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("findAndModifyImpl"); - } - - @Override - public String getMethodsInterceptor() { - return MONGDB_METHOD_INTERCET_CLASS; - } - - @Override - public boolean isOverrideArgs() { - return false; - } - }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("drop"); - } - - @Override - public String getMethodsInterceptor() { - return MONGDB_METHOD_INTERCET_CLASS; - } - - @Override - public boolean isOverrideArgs() { - return false; - } - }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("createIndex"); - } - - @Override - public String getMethodsInterceptor() { - return MONGDB_METHOD_INTERCET_CLASS; - } - - @Override - public boolean isOverrideArgs() { - return false; - } - } }; }