Escaping dot in resultAlias

philnate commented 7 years ago

I would like to query for a bunch of metrics which all share the same Type and some common name, e.g. I would have:

{
  "queries": [
    {
      "objectName": "my.mbean:name=cluster.*,*",
      "resultAlias": "%name%",
      "attributes": [
        "Value"
      ]
    }]
}

The metrics collected are supposed to be sent to graphite, but due to the org.jmxtrans.embedded.ResultNameStrategy the won't appear like cluster.some.hierarchy, rather than _cluster_somehierarchy. This is kind of breaking the graphite namespace as previously hierarchical metrics will share the same top level, as all levels are joined into one. This happens only if I use placeholders, if I would directly write the exact same name with dots, they're not replaced. Would it be possible to have the dot replacement optional?

philnate commented 7 years ago

@cyrille-leclerc you consider this something to be changed in embedded-jmxtrans?

cyrille-leclerc commented 7 years ago

@philnate I do apologize, I didn't see your previous message. This seem to be a very good idea.

Did you have a proposal?

I imagined to add a configuration attribute to org.jmxtrans.embedded.QueryAttribute and to add a configuration parameter on the query "resultNameStrategy":"dontEscapeDotsInObjectName".

Your query would look like:

{
  "queries": [
    {
      "objectName": "my.mbean:name=cluster.*,*",
      "resultAlias": "%name%",
      "resultNameStrategy":"dontEscapeDotsInObjectName",
      "attributes": [
        "Value"
      ]
    }]
}

philnate commented 7 years ago

@cyrille-leclerc that looks good to me and is basically what I envisioned, maybe a shorter resultNameStrategy would be nice :)

cyrille-leclerc commented 7 years ago

Please propose a value ;-)

philnate commented 7 years ago

Wouldn't it be sufficent to have something like dontEscapeDots? As it looks to me if there's an alias used, it resolves the aliases and then escapes it. So I don't see it limited to the ObjectName. Now while I'm thinking about it, escape doesn't seem appropriate either, as we don't escape it rather we replace it. Maybe dontReplaceDots?

cyrille-leclerc commented 7 years ago

Thanks @philnate I'll review ASAP. Other reviews are of course more than welcome.

philnate commented 7 years ago

Sorry to bug again, I guess you had not yet time to review this, @cyrille-leclerc?

cyrille-leclerc commented 7 years ago

Sorry for the delay, merged

cyrille-leclerc commented 7 years ago

@philnate can you please update the documentation at https://github.com/jmxtrans/embedded-jmxtrans/wiki ?

philnate commented 7 years ago

Thank you.

I've updated the following pages:

Please let me know if there's anything else I can help with. May I as well ask for an release of embedded-jmxtrans containing this change?

cyrille-leclerc commented 7 years ago

@philnate could you please test https://github.com/jmxtrans/embedded-jmxtrans/releases/tag/embedded-jmxtrans-1.2.0-beta-1 ?

Once I get your confirmation, I'll release as 1.2.0

philnate commented 7 years ago

@cyrille-leclerc works nicely.

philnate commented 7 years ago

Hi @cyrille-leclerc what's the status about releasing 1.2.0?

cyrille-leclerc commented 7 years ago

@philnate, I have a GPG signing key issue to upload 1.2.0 to oss.sonatype.org (I just changed my laptop). The artifact is available at https://github.com/jmxtrans/embedded-jmxtrans/releases/tag/embedded-jmxtrans-1.2.0 .

cyrille-leclerc commented 7 years ago

The deployment of 1.2.1 on MAven Central seem to have succeeded. I'll check in 10 hours.

https://github.com/jmxtrans/embedded-jmxtrans/releases/tag/embedded-jmxtrans-1.2.1

philnate commented 7 years ago

@cyrille-leclerc yes looks good. Thanks

jmxtrans / embedded-jmxtrans

Escaping dot in resultAlias #127