jmxtrans / jmxtrans-agent

Java Agent based JMX metrics exporter.
MIT License
178 stars 110 forks source link

Basic function support in outputwriter configuration #74

Closed simmel closed 8 years ago

simmel commented 8 years ago

I've configured my outputWriter to:

    <outputWriter class="org.jmxtrans.agent.GraphitePlainTextTcpOutputWriter">
        <host>graphite.domain.tld</host>
        <namePrefix>server.#escaped_canonical_hostname#.${jmxtrans_application_name:}</namePrefix>
    </outputWriter>

but what I receive is:

server.#escaped_canonical_hostname#.activemq.CurrentConnectionsCount 3 1457966635

Are basic functions supposed to work in the outputWriter config?

cyrille-leclerc commented 8 years ago

@simmel you are right, functions are not evaluated in the output writer config.

It would be a nice feature to add. Note that @kerlandsson have already added the PropertyResolver mechanism for the outputwriter config sections.

In the meantime, a solution may be to define this fragment in the resultAlias definition.

https://github.com/jmxtrans/jmxtrans-agent/blob/5f882a9e98c6ae0018018115d657ac42f9a06dc3/src/main/java/org/jmxtrans/agent/JmxTransConfigurationXmlLoader.java#L281-L281

simmel commented 8 years ago

On Mon, 2016-03-14 at 08:17:12 -0700, Cyrille Le Clerc wrote:

@simmel you are right, functions are not evaluated in the output writer config.

It would be a nice feature to add. Note that @kerlandsson have already added the PropertyResolver mechanism for the outputwriter config sections.

Yes, that's currently what we use today (Basically produce an ENV var with the value of #escaped_canonical_hostname# and use that).

Can I rename this issue to be a feature request for basic function support in the outputwriter section or do you want another one for that?

In the meantime, a solution may be to define this fragment in the resultAlias definition.

Agreed, but messy. Since we already have the ENV var in place we'd keep using that until this is resolved.

cyrille-leclerc commented 8 years ago

Thanks @simmel

kerlandsson commented 8 years ago

This is now implemented in commit 9d745c9503da54ca5af984be87e73c3b6faaa9fc.

@cyrille-leclerc a review of the (quite small) change would be appreciated.

@simmel if you are able to build the current master, test it and let me know how it works for you.

cyrille-leclerc commented 8 years ago

9d745c9 looks good to me! Tahnks

simmel commented 8 years ago

Works perfectly!

Thank you so much for this! Have a great weekend both of you!

simmel commented 8 years ago

Also, when is the next release coming? = D