jfrunit icon indicating copy to clipboard operation
jfrunit copied to clipboard

Generate constants for JFR event type names and attributes

Open gunnarmorling opened this issue 3 years ago • 13 comments

Allowing for a safe access to these literals in testing code.

gunnarmorling avatar Jul 18 '21 09:07 gunnarmorling

Hi, I'm not sure if this can be a solution but I created this POC https://github.com/Croway/jfrunit/tree/jfr-event-constants JfrUnitTest.shouldHaveGcAndSleepEvents contains the changes

Croway avatar Oct 14 '21 10:10 Croway

Hey @Croway, yeah, I think that looks pretty amazing! How did you do this, did you create some kind of code generator which produces all the constants?

In terms of what to generate exactly, instead of String constants, how would you feel about a more typed approach; something like this:

public class JfrEventTypes {
    public static final JfrEventType THREAD_START = ThreadStart.INSTANCE;
    ...
}
public class ThreadStart extends JfrEventType {
 
    public static final Attribute<ThreadStart, Instant> START_TIME = new Attribute<>();
    public static final Attribute<ThreadStart, Thread> EVENT_THREAD = new Attribute<>();
    ...
}

With the right method signatures, the assertion API could then be fully type-safe:

assertThat(jfrEvents).contains(
                event(JfrEventTypes.THREAD_START).with(ThreadStart.START_TIME, Instant.of(...)));

It's just a rough sketch, but the idea would be that with allows only attributes of the right event type and accepts only the right value type.

WDYT?

gunnarmorling avatar Oct 14 '21 14:10 gunnarmorling

Or maybe even this:

assertThat(jfrEvents).contains(
    JfrEventTypes.THREAD_START.withStartTime(Instant.of(...))
        .withThread(...));

I.e. generate event descriptors with the right methods for all their attributes. Even easier to use actually.

gunnarmorling avatar Oct 14 '21 14:10 gunnarmorling

yes, you are right, it's much better in this way, and I don't think it will be to hard to implement, in order to generate classes I created a simple code generator with freemarker that gets all the events from https://bestsolution-at.github.io/jfr-doc/openjdk-17.json (luckly there is a json api :smile: ), once I'll do the changes I'll share the code generator.

Croway avatar Oct 14 '21 14:10 Croway

Hi @gunnarmorling I implemented your first comment solution, JfrUnit should be backwards compatible with the older version (since I had failures with groovy tests), right now all test passes but if you use Attribute instead of strings the compiler will help.

This is the code generator app, https://github.com/Croway/jfr-constants-generator

Croway avatar Oct 15 '21 08:10 Croway

Very nice! So how would you like to proceed with this one? Moving the generator into the JfrUnit project? We may for instance run it as part of the JfrUnit build. WDYT?

gunnarmorling avatar Oct 15 '21 15:10 gunnarmorling

Sure, I can move the generator into the JfrUnit project, I don't know what is the best practice for this use case, run the generator as part of the build, or use it as a maven plugin.

Croway avatar Oct 16 '21 15:10 Croway

Yes, a Maven plug-in could work, or a simple annotation processor which then would be invoked by the compiler. It's not 100% a use case for an AP (given it's not reacting to any annotations within the JfrUnit code base), but might still do the trick for integrating the generator into the JfrUnit build (then also within IDEs for instance). Your call really :)

gunnarmorling avatar Oct 16 '21 15:10 gunnarmorling

I have created an annotation processor, you are right, not 100% a use case, but it works, these are the branches https://github.com/Croway/jfrunit/tree/jfr-event-cons-annotation-proc and https://github.com/Croway/jfr-constants-generator/tree/annotation-processor I think that we can proceed this way:

  • Transform jfrunit in a multi module maven project and add the generator or
  • Release the generator on maven central and use it as a dependency (I'm not experienced with releasing on maven central)

WDYT?

Croway avatar Oct 18 '21 09:10 Croway

I'd definitely prefer option a), making this a multi-module project. This will allow for quicker turnaround times after changes to the generator, and I think it just makes more sense to maintain everything in one place.

gunnarmorling avatar Oct 19 '21 20:10 gunnarmorling

On the style of the generated code, do you think the Attribute model is advantageous over the one with actual methods? I feel this would be nicer to use. WDYT?

gunnarmorling avatar Oct 19 '21 20:10 gunnarmorling

Hi @gunnarmorling, I pushed new commits into https://github.com/Croway/jfrunit/tree/jfr-event-cons-annotation-proc

  • I have converted jfrunit to a multi-module project, I need some help with naming and poms (I'm not sure how the release will work), right now I created 2 modules, core (ex jfrunit) and events-generator if you have any hint please let me know.
  • I managed to generate methods with right signature inside event classes, it works as you can see from JfrUnitTest#shouldHaveGcAndSleepEvents (line 47, 48) and it is much more clear and easy to use, but I need some time to integrate all functions and do a code cleanup.
  • I think that functions has and hasNot are pointless when using this typed approach, WDYT?

Croway avatar Oct 21 '21 07:10 Croway

Ok, that sounds great. I'd suggest you create a PR when you feel ready for it. Doesn't have to be polished yet, but it may help to discuss and converge on the overall approach (though I feel we're essentially there yet as per the discussion above). Thanks a lot!

gunnarmorling avatar Oct 21 '21 08:10 gunnarmorling