opentelemetry-java-instrumentation icon indicating copy to clipboard operation
opentelemetry-java-instrumentation copied to clipboard

Add some extra check to muzzle that allow it to check all TypeInstrumentation's transform methods are valid

Open 123liuziming opened this issue 1 year ago • 3 comments

Is your feature request related to a problem? Please describe.

For example, the rocketmq 5.0.0 does not support the PublishingMessageImplInstrumentation's transform method. The org.apache.rocketmq.client.java.impl.producer.PublishingSettings is only supported since 5.0.1, which is named "ProducerSettings" in 5.0.0. The muzzle check can not handle this. image image

Describe the solution you'd like

I want to make an enhancement to muzzle-check, to check all the mismatches in all TypeInstrumentation's transform methods. The effect of executing the ./gradlew :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle command should be as shown below

> Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.0 FAILED
FAILED MUZZLE VALIDATION: io.opentelemetry.javaagent.instrumentation.rocketmqclient.v5_0.RocketMqInstrumentationModule mismatches:
-- io.opentelemetry.javaagent.instrumentation.rocketmqclient.v5_0.ConsumeServiceInstrumentation$ConstructorAdvice:52 Missing method org.apache.rocketmq.client.java.impl.ClientImpl#getEndpoints()Lorg/apache/rocketmq/client/java/route/Endpoints;
-- <no-source> io.opentelemetry.javaagent.instrumentation.rocketmqclient.v5_0.PublishingMessageImplInstrumentation: method unmatched: (isConstructor() and isPublic() and hasParameter(hasTypes(with(0 matches erasure(name(equals(org.apache.rocketmq.client.apis.message.Message)))))) and hasParameter(hasTypes(with(1 matches erasure(name(equals(org.apache.rocketmq.client.java.impl.producer.PublishingSettings)))))) and hasParameter(hasTypes(with(2 matches erasure(is(boolean)))))),
-- <no-source> io.opentelemetry.javaagent.instrumentation.rocketmqclient.v5_0.ConsumeServiceInstrumentation: method unmatched: (isConstructor() and isPublic() and hasParameter(hasTypes(with(1 matches erasure(name(equals(org.apache.rocketmq.client.apis.consumer.MessageListener)))))) and hasParameter(hasTypes(with(3 matches erasure(name(equals(org.apache.rocketmq.client.java.hook.MessageInterceptor))))))),

> Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.6
Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.6 elapsed time: 3548ms

> Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.2
Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.2 elapsed time: 2911ms

> Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.3
Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.3 elapsed time: 2798ms

> Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.1 FAILED
FAILED MUZZLE VALIDATION: io.opentelemetry.javaagent.instrumentation.rocketmqclient.v5_0.RocketMqInstrumentationModule mismatches:
-- io.opentelemetry.javaagent.instrumentation.rocketmqclient.v5_0.ConsumeServiceInstrumentation$ConstructorAdvice:52 Missing method org.apache.rocketmq.client.java.impl.ClientImpl#getEndpoints()Lorg/apache/rocketmq/client/java/route/Endpoints;

> Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.5
Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.5 elapsed time: 6932ms

> Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.4
Task :instrumentation:rocketmq:rocketmq-client:rocketmq-client-5.0:javaagent:muzzle-AssertPass-org.apache.rocketmq-rocketmq-client-java-5.0.4 elapsed time: 2188ms

Describe alternatives you've considered

No response

Additional context

No response

123liuziming avatar Feb 23 '24 13:02 123liuziming

hi @123liuziming! can you describe the problem you're trying to solve a bit more? is this just a debugging option you want to add? I believe there are several instrumentations that use multiple method matchers to handle different library versions

trask avatar Feb 27 '24 22:02 trask

For example, the RocketMQ 5.0 plugin should use multiple method matchers to handle 5.0.0 version and [5.0.1) version, but the plugin does not do that, which results in some instrumentation failures when using RocketMQ version 5.0.0. I 'm trying to add some extra check while doing muzzle-check to solve the problem above. For example, if the result of muzzle-check of RocketMQ plugin is is shown like below:

-- <no-source> io.opentelemetry.javaagent.instrumentation.rocketmqclient.v5_0.PublishingMessageImplInstrumentation: method unmatched: (isConstructor() and isPublic() and hasParameter(hasTypes(with(0 matches erasure(name(equals(org.apache.rocketmq.client.apis.message.Message)))))) and hasParameter(hasTypes(with(1 matches erasure(name(equals(org.apache.rocketmq.client.java.impl.producer.PublishingSettings)))))) and hasParameter(hasTypes(with(2 matches erasure(is(boolean)))))),

We will find out that the transform in PublishingMessageImplInstrumentation is not right, there is no method that has the type like below: image

I believe the extra check in muzzle will help us find the problem if we transform a wrong method. Under these circumstances we should use multiple method matchers to handle different library versions.

123liuziming avatar Feb 28 '24 15:02 123liuziming

I 'm trying to add some extra check in io.opentelemetry.javaagent.tooling.muzzle.ClassLoaderMatcher. To match the class in RocketMQ and other library and the MethodMatcher in transform method of TypeInstrumentation. I 've finished a POC.

123liuziming avatar Feb 28 '24 15:02 123liuziming