Fix argument type name in postgresql plugin and add type name checking (#194)
* Fix argument type name of Array in postgresql-8.x-plugin and add type name checking in ArgumentTypeNameMatch and ReturnTypeNameMatch * Update docs/en/setup/service-agent/java-agent/Java-Plugin-Development-Guide.md * fix methods matchers in PgStatementInstrumentation Co-authored-by: 吴晟 Wu Sheng <wu.sheng@foxmail.com>
This commit is contained in:
ForestWang123
GitHub
parent
d468806f43
commit
372d754030
|
|
@ -20,6 +20,9 @@ Release Notes.
|
|||
* Support `-Dlog4j2.contextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector` in gRPC log report.
|
||||
* Fix tcnative libraries relocation for aarch64.
|
||||
* Add `plugin.jdbc.trace_sql_parameters` into Configuration Discovery Service.
|
||||
* Fix argument type name of Array in postgresql-8.x-plugin from java.lang.String[] to [Ljava.lang.String;
|
||||
* Add type name checking in ArgumentTypeNameMatch and ReturnTypeNameMatch
|
||||
* Highlight ArgumentTypeNameMatch and ReturnTypeNameMatch type naming rule in docs/en/setup/service-agent/java-agent/Java-Plugin-Development-Guide.md
|
||||
|
||||
#### Documentation
|
||||
|
||||
|
|
|
|||
|
|
@ -46,6 +46,8 @@ public class ArgumentTypeNameMatch implements ElementMatcher<MethodDescription>
|
|||
* @param argumentTypeName target argument type
|
||||
*/
|
||||
private ArgumentTypeNameMatch(int index, String argumentTypeName) {
|
||||
ArrayTypeNameChecker.check(argumentTypeName);
|
||||
|
||||
this.index = index;
|
||||
this.argumentTypeName = argumentTypeName;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,28 @@
|
|||
/*
|
||||
* 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.skywalking.apm.agent.core.plugin.bytebuddy;
|
||||
|
||||
public class ArrayTypeNameChecker {
|
||||
|
||||
public static void check(String typeName) {
|
||||
if (typeName.endsWith("[]")) {
|
||||
throw new IllegalArgumentException("Please use [Lxxx; to define an Array type, and ref to JVM Specification for details");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -41,6 +41,7 @@ public class ReturnTypeNameMatch implements ElementMatcher<MethodDescription> {
|
|||
* @param returnTypeName target return type
|
||||
*/
|
||||
private ReturnTypeNameMatch(String returnTypeName) {
|
||||
ArrayTypeNameChecker.check(returnTypeName);
|
||||
this.returnTypeName = returnTypeName;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,44 @@
|
|||
/*
|
||||
* 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.skywalking.apm.agent.core.plugin.bytebuddy;
|
||||
|
||||
import net.bytebuddy.description.method.MethodDescription;
|
||||
import net.bytebuddy.matcher.ElementMatcher;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Test;
|
||||
|
||||
public class ArgumentTypeNameMatchTest {
|
||||
|
||||
@Test
|
||||
public void testMatch() throws Exception {
|
||||
final ElementMatcher<MethodDescription> matcherPerson = ArgumentTypeNameMatch.takesArgumentWithType(0, "org.apache.skywalking.apm.agent.core.plugin.bytebuddy.Person");
|
||||
Assert.assertTrue(matcherPerson.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("isOlderThan", Person.class))));
|
||||
Assert.assertFalse(matcherPerson.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("isMemberOf", Person[].class))));
|
||||
|
||||
final ElementMatcher<MethodDescription> matcherPersonArray = ArgumentTypeNameMatch.takesArgumentWithType(0, "[Lorg.apache.skywalking.apm.agent.core.plugin.bytebuddy.Person;");
|
||||
Assert.assertTrue(matcherPersonArray.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("isMemberOf", Person[].class))));
|
||||
Assert.assertFalse(matcherPersonArray.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("isOlderThan", Person.class))));
|
||||
}
|
||||
|
||||
@Test (expected = IllegalArgumentException.class)
|
||||
public void testMatchWithException() {
|
||||
ArgumentTypeNameMatch.takesArgumentWithType(0, "org.apache.skywalking.apm.agent.core.plugin.bytebuddy.Person[]");
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -40,4 +40,26 @@ public class Person {
|
|||
return age;
|
||||
}
|
||||
|
||||
public boolean isOlderThan(Person anotherPerson) {
|
||||
return this.age > anotherPerson.age;
|
||||
}
|
||||
|
||||
public boolean isMemberOf(Person[] persons) {
|
||||
for (Person person: persons) {
|
||||
if (person == this) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
public Person[] getFriends() {
|
||||
return null;
|
||||
}
|
||||
|
||||
public Person getBestFriend() {
|
||||
return null;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -27,9 +27,17 @@ public class ReturnTypeNameMatchTest {
|
|||
|
||||
@Test
|
||||
public void testMatch() throws Exception {
|
||||
final ElementMatcher<MethodDescription> matcher = ReturnTypeNameMatch.returnsWithType("java.lang.String");
|
||||
Assert.assertTrue(matcher.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("getName"))));
|
||||
Assert.assertFalse(matcher.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("getAge"))));
|
||||
final ElementMatcher<MethodDescription> matcherString = ReturnTypeNameMatch.returnsWithType("java.lang.String");
|
||||
Assert.assertTrue(matcherString.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("getName"))));
|
||||
Assert.assertFalse(matcherString.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("getAge"))));
|
||||
|
||||
final ElementMatcher<MethodDescription> matcherPersonArray = ReturnTypeNameMatch.returnsWithType("[Lorg.apache.skywalking.apm.agent.core.plugin.bytebuddy.Person;");
|
||||
Assert.assertTrue(matcherPersonArray.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("getFriends"))));
|
||||
Assert.assertFalse(matcherPersonArray.matches(new MethodDescription.ForLoadedMethod(Person.class.getMethod("getBestFriend"))));
|
||||
}
|
||||
|
||||
@Test (expected = IllegalArgumentException.class)
|
||||
public void testMatchWithException() {
|
||||
ReturnTypeNameMatch.returnsWithType("org.apache.skywalking.apm.agent.core.plugin.bytebuddy.Person[]");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -46,7 +46,7 @@ public class ConnectionInstrumentation extends ClassInstanceMethodsEnhancePlugin
|
|||
|
||||
private static final String PREPARE_STATEMENT_METHOD_WITH_STRING_ARRAY_INTERCEPTOR_CLASS = "org.apache.skywalking.apm.plugin.jdbc.postgresql.JDBCPrepareStatementWithStringArrayInterceptor";
|
||||
public static final String PG_CONNECTION_ENHANCE_CLASS = "org.postgresql.jdbc.PgConnection";
|
||||
public static final String STRING_ARRAY_ARGUMENT_TYPE = "java.lang.String[]";
|
||||
public static final String STRING_ARRAY_ARGUMENT_TYPE = "[Ljava.lang.String;";
|
||||
public static final String PG_JDBC42_CONNECTION_ENHANCE_CLASS = "org.postgresql.jdbc42.Jdbc42Connection";
|
||||
public static final String PG_JDBC3_CONNECTION_ENHANCE_CLASS = "org.postgresql.jdbc3g.Jdbc3gConnection";
|
||||
public static final String PG_JDBC4_CONNECTION_ENHANCE_CLASS = "org.postgresql.jdbc4.Jdbc4Connection";
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInst
|
|||
import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
|
||||
|
||||
import static net.bytebuddy.matcher.ElementMatchers.named;
|
||||
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
|
||||
import static org.apache.skywalking.apm.agent.core.plugin.bytebuddy.ArgumentTypeNameMatch.takesArgumentWithType;
|
||||
import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
|
||||
import static org.apache.skywalking.apm.plugin.jdbc.postgresql.Variables.PG_STATEMENT_EXECUTE_METHOD_INTERCEPTOR;
|
||||
|
|
@ -43,12 +44,11 @@ public class PgStatementInstrumentation extends ClassInstanceMethodsEnhancePlugi
|
|||
new InstanceMethodsInterceptPoint() {
|
||||
@Override
|
||||
public ElementMatcher<MethodDescription> getMethodsMatcher() {
|
||||
return named("execute").and(takesArgumentWithType(0, "java.lang.String"))
|
||||
.or(named("execute").and(takesArgumentWithType(0, "java.lang.String[]")))
|
||||
.or(named("executeQuery"))
|
||||
.or(named("executeUpdate").and(takesArgumentWithType(0, "java.lang.String[]")))
|
||||
.or(named("executeUpdate").and(takesArgumentWithType(0, "java.lang.String")))
|
||||
.or(named("executeLargeUpdate"));
|
||||
return named("execute").and(takesArgumentWithType(0, "java.lang.String")).and(takesArguments(1))
|
||||
.or(named("execute").and(takesArgumentWithType(1, "[Ljava.lang.String;")))
|
||||
.or(named("executeQuery"))
|
||||
.or(named("executeUpdate").and(takesArgumentWithType(0, "java.lang.String")).and(takesArguments(1)))
|
||||
.or(named("executeUpdate").and(takesArgumentWithType(1, "[Ljava.lang.String;")));
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
|||
|
|
@ -0,0 +1,47 @@
|
|||
/*
|
||||
* 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.skywalking.apm.plugin.jdbc.postgresql.define;
|
||||
|
||||
import net.bytebuddy.description.method.MethodDescription;
|
||||
import net.bytebuddy.matcher.ElementMatcher;
|
||||
import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.postgresql.jdbc.PgConnection;
|
||||
|
||||
import java.lang.reflect.Array;
|
||||
|
||||
public class ConnectionInstrumentationTest {
|
||||
private ConnectionInstrumentation connectionInstrumentation;
|
||||
|
||||
@Before
|
||||
public void setUp() {
|
||||
connectionInstrumentation = new ConnectionInstrumentation();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testMethodMatch() throws Exception {
|
||||
final InstanceMethodsInterceptPoint[] interceptPoints = connectionInstrumentation.getInstanceMethodsInterceptPoints();
|
||||
Assert.assertEquals(5, interceptPoints.length);
|
||||
final ElementMatcher<MethodDescription> arrayArgumentMatcher = interceptPoints[1].getMethodsMatcher();
|
||||
final MethodDescription method = new MethodDescription.ForLoadedMethod(PgConnection.class.getMethod("prepareStatement", String.class, Array.newInstance(String.class, 0).getClass()));
|
||||
Assert.assertTrue(arrayArgumentMatcher.matches(method));
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,78 @@
|
|||
/*
|
||||
* 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.skywalking.apm.plugin.jdbc.postgresql.define;
|
||||
|
||||
import net.bytebuddy.description.method.MethodDescription;
|
||||
import net.bytebuddy.matcher.ElementMatcher;
|
||||
import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.postgresql.jdbc.PgStatement;
|
||||
|
||||
import java.lang.reflect.Array;
|
||||
|
||||
public class PgStatementInstrumentationTest {
|
||||
private PgStatementInstrumentation pgStatementInstrumentation;
|
||||
|
||||
@Before
|
||||
public void setUp() {
|
||||
pgStatementInstrumentation = new PgStatementInstrumentation();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testMethodMatch() throws Exception {
|
||||
final InstanceMethodsInterceptPoint[] interceptPoints = pgStatementInstrumentation.getInstanceMethodsInterceptPoints();
|
||||
Assert.assertEquals(1, interceptPoints.length);
|
||||
|
||||
final ElementMatcher<MethodDescription> arrayArgumentMatcher = interceptPoints[0].getMethodsMatcher();
|
||||
final MethodDescription methodExecuteString = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("execute", String.class));
|
||||
final MethodDescription methodExecuteStringInt = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("execute", String.class, int.class));
|
||||
final MethodDescription methodExecuteStringIntArray = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("execute", String.class, Array.newInstance(int.class, 0).getClass()));
|
||||
final MethodDescription methodExecuteStringStringArray = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("execute", String.class, Array.newInstance(String.class, 0).getClass()));
|
||||
|
||||
Assert.assertTrue(arrayArgumentMatcher.matches(methodExecuteString));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteStringInt));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteStringIntArray));
|
||||
Assert.assertTrue(arrayArgumentMatcher.matches(methodExecuteStringStringArray));
|
||||
|
||||
final MethodDescription methodExecuteUpdateString = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeUpdate", String.class));
|
||||
final MethodDescription methodExecuteUpdateStringInt = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeUpdate", String.class, int.class));
|
||||
final MethodDescription methodExecuteUpdateStringIntArray = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeUpdate", String.class, Array.newInstance(int.class, 0).getClass()));
|
||||
final MethodDescription methodExecuteUpdateStringStringArray = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeUpdate", String.class, Array.newInstance(String.class, 0).getClass()));
|
||||
Assert.assertTrue(arrayArgumentMatcher.matches(methodExecuteUpdateString));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteUpdateStringInt));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteUpdateStringIntArray));
|
||||
Assert.assertTrue(arrayArgumentMatcher.matches(methodExecuteUpdateStringStringArray));
|
||||
|
||||
final MethodDescription methodExecuteLargeUpdateString = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeLargeUpdate", String.class));
|
||||
final MethodDescription methodExecuteLargeUpdateStringInt = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeLargeUpdate", String.class, int.class));
|
||||
final MethodDescription methodExecuteLargeUpdateStringIntArray = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeLargeUpdate", String.class, Array.newInstance(int.class, 0).getClass()));
|
||||
final MethodDescription methodExecuteLargeUpdateStringStringArray = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeLargeUpdate", String.class, Array.newInstance(String.class, 0).getClass()));
|
||||
final MethodDescription methodExecuteLargeUpdateInvalid = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeLargeUpdate"));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteLargeUpdateString));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteLargeUpdateStringInt));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteLargeUpdateStringIntArray));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteLargeUpdateStringStringArray));
|
||||
Assert.assertFalse(arrayArgumentMatcher.matches(methodExecuteLargeUpdateInvalid));
|
||||
|
||||
final MethodDescription methodExecuteQuery = new MethodDescription.ForLoadedMethod(PgStatement.class.getMethod("executeQuery", String.class));
|
||||
Assert.assertTrue(arrayArgumentMatcher.matches(methodExecuteQuery));
|
||||
}
|
||||
}
|
||||
|
|
@ -297,7 +297,16 @@ public interface InstanceMethodsInterceptPoint {
|
|||
}
|
||||
```
|
||||
You may also use `Matcher` to set the target methods. Return **true** in `isOverrideArgs`, if you want to change the argument
|
||||
ref in interceptor.
|
||||
ref in interceptor.
|
||||
Please refer to [bytebuddy](https://bytebuddy.net/#/) for details of defining `ElementMatcher`.
|
||||
|
||||
In Skywalking, we provide 3 classes to facilitate `ElementMatcher` definition:
|
||||
* `AnnotationTypeNameMatch`: Check on whether there is a certain annotation in the target method.
|
||||
* `ReturnTypeNameMatch`: Check the return type name (package name + `.` + class name) of the target method.
|
||||
* `ArgumentTypeNameMatch`: Check on the argument index and the type name (package name + `.` + class name) of the target method.
|
||||
|
||||
**Attention**:
|
||||
* In case of using `ReturnTypeNameMatch` and `ArgumentTypeNameMatch`, use [Lxxx; (Java file format defined in [JVM Specification](https://docs.oracle.com/javase/specs/jvms/se8/html/)) to define an Array type. For example, you should write `[Ljava.lang.String;` for `java.lang.String[]`.
|
||||
|
||||
The following sections will tell you how to implement the interceptor.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue