RFR: 8223066: "jfr metadata" output the @Name annotation twice.

[Erik Gahlin]

Hi Chihiro,

Thanks for reporting the issue, @Name should not be printed twice.

Let me give you some background information. I think there are two 
issues at hand:

1) The Name annotation should probably not be persisted. This can be 
achieved by removing the MetadataDefinition from the class. I did try 
that, but I noticed several tests failing, in particular TestName which 
explicitly checks that @Name is persisted. The purpose @Name is to 
override the default name, which can then be retrieved from 
ValueDescriptor::getName, EventType::getName and 
SettingDescriptor::getName. The annotation is not needed on the consumer 
side as the name is already persisted elsewhere.

I remember discussing this several times during the design, but for some 
reason we came to the conclusion to persist it, probably to not confuse 
users if it didn't behave like @Label and @Description. Later,  
Metadatadefinition was removed from @Threshold, @Periodic, @Enabled and 
@StackTrace. At that time, it should probably have been removed from 
@Name as well. Maybe we should go ahead and remove it now, but I will 
have to think about the implications first.

2) The reason PrettyWriter::printType adds @Name is because there is no 
good way to include package/namespace in the printout. Events that lack 
a namespace should not get the annotation, that's why there is a check 
for "." It's not because the values are primitives.

I can see two ways to proceed. Either add a check in printType so @Name 
is only printed if it doesn't already exists, i.e.

if (t.getAnnotation(Name.class) == null) {
   if (index != -1) {
     println("@Name(\"" + typeName + "\")");
   }
}

  or remove @MetadataDefinition from the Name class and make adjustments 
to tests etc. If you chose the latter, I like a week to think it over, 
investigate the implications and discuss it with other people, before I 
can say if it is the right approach or not.

Thanks
Erik

Hi,
>
> I fixed an issue that the @Name annotation was printed twice as 
> followings in "jfr metadata" output.
>
> @Name("jdk.ActiveSetting")
> @Name("jdk.ActiveSetting")
> @Label("Recording Setting")
> @Category("Flight Recorder")
> class ActiveSetting extends jdk.jfr.Event {
>
>
> Could you reveiew this please?
>
> webrev:
> http://cr.openjdk.java.net/~cito/JDK-8223066/webrev.00/ 
> <http://cr.openjdk.java.net/%7Ecito/JDK-8223066/webrev.00/>
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8223066
>
> I have passed all the tests in test-tier1.
>
> Best regards,
> Chihiro