RFR: 7033: Allow overriding the id (name) of the event type [v2]

Marcus Hirt hirt at openjdk.java.net
Tue Jan 5 23:15:59 UTC 2021


On Mon, 4 Jan 2021 16:40:03 GMT, Gunnar Morling <github.com+28612+gunnarmorling at openjdk.org> wrote:

>> @thegreystone et al., this PR proposes two changes to the names of generated event types and their fields:
>> 
>> * An explicit event type name is derived from the name given in the configuration, instead of using the class name; this is nicer to work with e.g. when enabling/disabling event types for recordings
>> * the "field" prefix is removed from event type field names; when examining e.g. `RecordedEvent` instances, names are nicer to look at that way IMO
>> 
>> Could you log an issue for this change if you agree it's desirable? Thanks! I ran into these issues while using JMC Agent for [tracking SQL statements](https://twitter.com/gunnarmorling/status/1333152245463064579); note the odd "fieldSQLQuery" name marked on the left; the event type name is already adjusted in this screenshot as per this PR here.
>
> Gunnar Morling has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
> 
>   7033: Nicer names for event types and fields

Marked as reviewed by hirt (Lead).

agent/src/main/java/org/openjdk/jmc/agent/jfrnext/impl/JFRNextEventClassGenerator.java line 235:

> 233: 
> 234: 		// Name
> 235: 		av = cw.visitAnnotation("Ljdk/jfr/Name;", true);

I'd be careful here. The event name is basically the event id. I'd certainly not add Event to the end, or derive it from the event name - the event names for OpenJDK events are typically things like jdk.ClassLoaderStatistics. If anything, it's the id attribute in the templates that should be put in here. That said, I think defaulting to class is fine.

To better correspond to JFR 2.0, we may want to rename "name" to "label" to avoid confusion. I'll do that in a separate PR.

-------------

PR: https://git.openjdk.java.net/jmc/pull/170


More information about the jmc-dev mailing list