writeQueryResult method update

[evmin]

This is an enhancement request.

It would be so much easier to further develop the API if the OutputWriter interface method

    /**
     * @param metricName
     * @param metricType see {@link Query#type}
     * @param value
     * @throws IOException
     */
    void writeQueryResult(@Nonnull String metricName, @Nullable String metricType, @Nullable Object value) throws IOException;

Could be replaced with

   /**
     * @param metricName
     * @param queryResult see {@link QueryResult}
     * @param value
     * @throws IOException
     */
    void writeQueryResult(@Nonnull String metricName, @Nullable QueryResult queryResult) throws IOException;

In that case the backward incompatible changes to the API (such as including extra fields, which are required by the new Writers) would be so much easier to implement.

Happy to assist with rewriting.

[cyrille-leclerc]

Hi @evmin, I trust you but I don't understand yet how it would work with indexed values... I would need to test

Rather than deleting the existing writeQueryResult(...) method, I am thinking about introducing an OutputWriterV2 interface with this new writeQueryResult(...) and deprecating OutputWriterV1 to let people have to change their code.

Could you initiate a branch as a PR?

[evmin]

I might have missed the specifics of the indexed values - thank you for pointing it out.

Also, I like the v2 API idea as v2 opens up for possibility of other potential improvements:

• introducing dynamic options on Query, where any unknown to date attributes can be sources into settings like HashMap and passed onto the writers
• optimised QueryResult object
• access to the underlying Query properties

It sounds a bit more invasive though than just retrofitting one method. Would you like to map out the new interface yourself or you are OK with me just start on the new branch (I am just mindful that I might not be aware of all specifics, see the indexed values above)?

[cyrille-leclerc]

@evmin could you initiate the work on a PR? Maybe it will be the opportunity to cut a v2.0 of the plugin.