newrelic / newrelic-java-agent

The New Relic Java agent
Apache License 2.0
202 stars 143 forks source link

WeavedMethod annotation on constructor #1153

Closed thiagosardinha closed 1 year ago

thiagosardinha commented 1 year ago

I have a Java 11 application with Dropwizard framework that implements some Quartz jobs.

After bumping Dropwizard's version from 2.1.1 to 2.1.4 the application stopped working when using the New Relic Java agent (tested with versions 7.10.0 and 8.0.0). After a lot of debugging, I tracked down that Dropwizard's new version changed its proxy implementation from javassist to bytebuddy and the latter has a validation that does not allow elements with annotations that don't target it.

Problem is, New Relic Java agent annotates my Quartz jobs implementations' constructors with the @WeaveMethod annotation, which bytebuddy considers illegal state since the annotation targets only methods. This is where it fails.

It's so many moving parts that I'm not even sure the issue is here, but seems that bytebuddy's validation sort of makes sense, so here I am.

Note, the job classes are also annotated with the @InstrumentedMethod annotation, which I assume would fail as well.

Expected Behavior

Maybe include constructors as target in the @WeaveMethod annotation. Or use another annotation specific for constructors so targets are correct for the included annotations.

Troubleshooting or NR Diag results

Exception that is raised by bytebuddy in this validation

java.lang.IllegalStateException: Cannot add @com.newrelic.agent.instrumentation.WeavedMethod(source={"com.newrelic.instrumentation.quartz-2.0.0"}) on public test.SomeImplementationOfQuartzJob$ByteBuddy$CLCWj0h4(java.util.Optional)
    at net.bytebuddy.dynamic.scaffold.InstrumentedType$Default.validated(InstrumentedType.java:1793)

debugging the bytebuddy shows the included annotations WhatsApp Image 2023-02-14 at 18 28 12 validation

annotated class snippet

import org.quartz.Job;

import java.util.Optional;

public class SomeImplementationOfQuartzJob implements Job {

    private final Optional<MetricsReporter> metricsReporter;

    public SomeImplementationOfQuartzJob(Optional<MetricsReporter> metricsReporter)
    {
        this.metricsReporter = metricsReporter;
    }

    // ...
}

metrics reporters is just a wrapper to new relic

public class MetricsReporter {

    private static final String NEW_RELIC_TRANSACTION_NAME_PREFIX = "/";

    public void reportTransaction(String category, String transactionName) {
        NewRelic.setTransactionName(category, NEW_RELIC_TRANSACTION_NAME_PREFIX + transactionName);
    }

    public void reportCustomEvent(String eventName, Map<String, Object> customParams) {
        NewRelic.getAgent().getInsights().recordCustomEvent(eventName, customParams);
    }

    public void reportTransaction(String category, String transactionName, Map<String, String> customParams) {
        NewRelic.setTransactionName(category, NEW_RELIC_TRANSACTION_NAME_PREFIX + transactionName);

        customParams.forEach(NewRelic::addCustomParameter);
    }
}

Steps to Reproduce

as I said, started happening to me after bumping Dropwizard's version to 2.1.4, but that's just because they switched from using javassist to bytebuddy which has the validation I mentioned.

Basically, using NR java agent in an Dropwizard 2.1.4 application with a class implementing org.quartz.Job.

Your Environment

Additional context

More details on Dropwizard's change to bytebuddy: https://github.com/dropwizard/dropwizard/pull/5748

workato-integration[bot] commented 1 year ago

https://issues.newrelic.com/browse/NEWRELIC-6861

meiao commented 1 year ago

Hi @thiagosardinha, thanks for the investigation. Could you test a custom agent to see if it fixes this problem?

It is being generated now and should be available in a few minutes at: https://github.com/newrelic/newrelic-java-agent/actions/runs/4178835318 As soon as it is ready, in that page, under "Artifacts" there will be an entry "custom-newrelic-jar" which is a zip file containing the agent jar.

thiagosardinha commented 1 year ago

Hi @meiao. Thanks for the quick response!

Tested the custom jar and seems to have worked for @WeavedMethod, but got the same error for @InstrumentedMethod:

java.lang.IllegalStateException: Cannot add @com.newrelic.agent.instrumentation.InstrumentedMethod(dispatcher=false, instrumentationTypes={WeaveInstrumentation}, instrumentationNames={"com.newrelic.instrumentation.quartz-2.0.0"})  on public test.SomeImplementationOfQuartzJob$ByteBuddy$CLCWj0h4(java.util.Optional)
    at net.bytebuddy.dynamic.scaffold.InstrumentedType$Default.validated(InstrumentedType.java:1793)

My guess is the same fix would have to be applied to it too.

meiao commented 1 year ago

Sorry, I overlooked your comment at the bottom about @InstrumentedMethod. Another custom jar is building, can soon be retrieved at https://github.com/newrelic/newrelic-java-agent/actions/runs/4185251493

thiagosardinha commented 1 year ago

That worked! Thanks!