Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not transform Mockito-generated classes #7048

Conversation

nikita-tkachenko-datadog
Copy link
Contributor

@nikita-tkachenko-datadog nikita-tkachenko-datadog commented May 17, 2024

What Does This Do

Adds the common package for Mockito-generated classes to the list of global ignores.

Motivation

Whenever a class is mocked with Mockito, a new class is generated that replaces the original class' methods with the mocked ones.

If the tracer contains instrumentations that transform the original class, the same instrumentations will most likely be applied to the mocked class as it extends the original one.

Transforming the mocked class' code leads to errors: some instrumentations, when executed a transformed method, call another method of the same instance. Mockito does not handle this well: if a mocked method is called from inside another mocked method, Mockito's internal state gets corrupted.

The solution is to avoid transforming Mockito-generated classes, both because it fixes the errors and because tracing mock methods makes little sense.

Jira ticket: CIVIS-10060

Copy link
Contributor

@mcculls mcculls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CombiningTransformerBuilder is not the place to add this kind of exclusion because it will degrade performance by adding an extra check on the name for everything - even when we already know the class doesn't start with that package.

Since you want to exclude everything under a package the simplest way is to add it to https://github.com/DataDog/dd-trace-java/blob/master/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie where we maintain a list of packages and classes to ignore for performance reasons (because we know they are uninteresting) or they should never be instrumented because it would break things.

You should just need to add:

1 org.mockito.codegen.*

Note if you wanted to exclude generated/proxy classes which didn't have a common package, but had another recognizable token in their name then you can do that in https://github.com/DataDog/dd-trace-java/blob/master/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/ProxyClassIgnores.java

@nikita-tkachenko-datadog nikita-tkachenko-datadog force-pushed the nikita-tkachenko/do-not-instrument-mockito-mocks branch from cdfde4e to b953a7f Compare May 17, 2024 14:42
@nikita-tkachenko-datadog nikita-tkachenko-datadog changed the title Update instrumentation logic to never transform Mockito-generated classes Do not transform Mockito-generated classes May 17, 2024
@nikita-tkachenko-datadog nikita-tkachenko-datadog marked this pull request as ready for review May 19, 2024 20:25
@nikita-tkachenko-datadog nikita-tkachenko-datadog requested a review from a team as a code owner May 19, 2024 20:25
Copy link
Contributor

@mcculls mcculls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@nikita-tkachenko-datadog nikita-tkachenko-datadog merged commit 57369e8 into master May 21, 2024
83 of 88 checks passed
@nikita-tkachenko-datadog nikita-tkachenko-datadog deleted the nikita-tkachenko/do-not-instrument-mockito-mocks branch May 21, 2024 10:06
@github-actions github-actions bot added this to the 1.35.0 milestone May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: ci visibility Continuous Integration Visibility type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants