[Rev 02] RFR: 6725: Formalize agent probe definition XML schema and enforce validations

Alex Macdonald aptmac at openjdk.java.net
Mon Apr 6 17:01:14 UTC 2020


On Thu, 26 Mar 2020 21:25:06 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> This pr implements [JMC-6725: Formalize agent probe definition XML schema and enforce
>> validations](https://bugs.openjdk.java.net/browse/JMC-6725). XMLs supplied by users are validated against
>> [`jfrprobes_schema.xsd`](https://github.com/tabjy/jmc/blob/a4da862b744f407f3262f006e302515c142e9482/agent/src/main/resources/org/openjdk/jmc/agent/impl/jfrprobes_schema.xsd)
>> on registry creation and modification.  Notable changes in configuration probes format are:
>> - `<parameter>` elements now should be enclosed by a `<parameters>` element, and
>> - `<field>` elements now should be enclosed by a `<fields>` element
>> 
>> Type checking is enforced on most elements. Additionally, the following types are restricted with regular expressions
>> checking elements' text node contents:
>> - `classType` for `<class>` and `<converter>` elements
>> - `methodNameType` for `<name>` elements
>> - `descriptorType` for `<descriptor>` elements
>> -  `pathType` for `<path>` elements, and
>> - `expressionType` for `<expression>` elements
>> 
>> Please test, especially on regex, as I'm not entirely confident they cover every cases and don't yield false negatives.
>
> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix TestRetrieveCurrentTransforms

This looks good to me, I do have a question and a request though.

1. Mainly out of curiosity, how are you doing your XML formatting? Some of the `<description>`s were changed to be
mulit-line, but only have a single word in the new line.. it initially looked odd to me but I guess it makes sense.

2. Could you add a unit test or two verifying the behaviour of an invalid probe definition? The current unit tests only
verify the happy-path, and while the regex look good to me I would feel better if there was a test case for some
invalid patterns.

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

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


More information about the jmc-dev mailing list