kieker-monitoring / kieker-source-instrumentation

This projects provides instrumentation by changing the source code of the project. Thereby, monitoring of method execution durations can be done with minimal overhead.
Apache License 2.0
0 stars 0 forks source link

Do not use private static attributes for instrumentation of Interfaces #1

Closed stro18 closed 2 years ago

stro18 commented 2 years ago

Suggested Solution Use static instead of private static

Note Might already be implemented and the implementation of Peass might be responsible for the false instrumentation.

DaGeRe commented 2 years ago

Thanks for the issue.

This should already work, as checked by https://github.com/DaGeRe/kieker-source-instrumentation-dagere/blob/main/kieker-source-instrumentation-library/src/test/java/net/kieker/sourceinstrumentation/it/SourceInstrumentationInterfaceIT.java (and the example interface is in https://github.com/DaGeRe/kieker-source-instrumentation-dagere/blob/main/kieker-source-instrumentation-library/src/test/resources/project_2_interface/src/main/java/de/peass/SomeInterface.java).

I do not see that there is a special configuration for this, the KiekerFieldAdder just adds static final instead private final on all interfaces: https://github.com/DaGeRe/kieker-source-instrumentation-dagere/blob/f37cd0d1acb381470315a95a993ca9a3d6b92692/kieker-source-instrumentation-library/src/main/java/net/kieker/sourceinstrumentation/instrument/KiekerFieldAdder.java#L68

Might your concrete use case contain other aspects, e.g. inner classes or something like this? Could you provide the full class where the problem occurs?

stro18 commented 2 years ago

The error only occurred for measurement instrumentation of root cause analysis. An example interface with that error was https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/JarScanFilter.java

DaGeRe commented 2 years ago

Using DurationRecord for instrumentation in the default configuration also works: https://github.com/DaGeRe/kieker-source-instrumentation-dagere/commit/59fb09dea6b1cd44411e53b02b88d721c0617b21 (and also in other configurations).

Which concrete tomcat commit and unit test case did you use for the root cause analysis?

stro18 commented 2 years ago

For instance, I executed:

./peasstomcat searchcauseParameterized 314568fa450680711f12f01ee050f2166f4263bd org.apache.catalina.connector.TestResponse#testSetLocale04

For that command, the error occurred in the file https://github.com/stro18/tomcat/blob/314568fa450680711f12f01ee050f2166f4263bd/java/org/apache/coyote/ProtocolHandler.java

stro18 commented 2 years ago

The following lines caused the error:

private static int _kieker_sourceInstrumentation_createCounter0;

private static long _kieker_sourceInstrumentation_createSum0;
DaGeRe commented 2 years ago

Thanks for the command, with this command, I can reproduce the problem. I'll fix it in the next days.

DaGeRe commented 2 years ago

This should be fixed since https://github.com/DaGeRe/kieker-source-instrumentation-dagere/commit/2bc48ed73dcfa52ceb75667ee373339108a1ce88 For my local tests, this worked for 314568fa450680711f12f01ee050f2166f4263bd and org.apache.catalina.connector.TestResponse#testSetLocale04. Does this also work for you?

stro18 commented 2 years ago

I can confirm that it works.