Graphite tag support #1806

fitzoh · 2020-01-18T03:42:41Z

~~Super Janky~~ Quite reasonable (@jkschneider edit) support for #491.

Adds an alternative dimensional name mapper and naming convention for graphite that supports tagging.

...micrometer-registry-graphite/src/main/java/io/micrometer/graphite/GraphiteTagNameMapper.java

...ometer-registry-graphite/src/test/java/io/micrometer/graphite/GraphiteTagNameMapperTest.java

...micrometer-registry-graphite/src/main/java/io/micrometer/graphite/GraphiteTagNameMapper.java

...ometer-registry-graphite/src/test/java/io/micrometer/graphite/GraphiteTagNameMapperTest.java

...micrometer-registry-graphite/src/main/java/io/micrometer/graphite/GraphiteTagNameMapper.java

...ometer-registry-graphite/src/test/java/io/micrometer/graphite/GraphiteTagNameMapperTest.java

…s + tests

… handled by the naming convention tests)

...istry-graphite/src/main/java/io/micrometer/graphite/GraphiteDimensionalNamingConvention.java

jkschneider · 2020-01-27T13:45:08Z

Thanks for the cool feature @fitzoh!

davide-baldo · 2020-05-07T11:24:53Z

This PR seems broken, it always generate broken names due to DropWizard behavior.

From DropWizard, GraphiteReporter [1]:
graphite.send(prefix(name, type.getCode()), format(value), timestamp);
where
name="my.metric;tag=value"
type.getCode() = "p99"

final string sent to graphite: "my.metric;tag=valuep99"

Unless we patch DropWizard side i can't see how this code may work

[1] https://github.com/dropwizard/metrics/blob/v4.1.7/metrics-graphite/src/main/java/com/codahale/metrics/graphite/GraphiteReporter.java#L351

shakuzen · 2020-05-08T04:09:22Z

@workless could you please open a new issue so we can take a look at it? /cc @fitzoh

…g-boot to 2.3.12 While upgrading the spring-boot by transitively upgrading the orca version in kayenta and enforcing the dependencies, the kayenta-integration-tests:GraphiteStandaloneCanaryAnalysisTest failed with below error: ``` java.lang.AssertionError: 1 expectation failed. JSON path executionStatus doesn't match. Expected: is "SUCCEEDED" Actual: TERMINAL at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490) at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72) at org.codehaus.groovy.reflection.CachedConstructor.doConstructorInvoke(CachedConstructor.java:59) at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrap.callConstructor(ConstructorSite.java:84) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:277) at io.restassured.internal.ResponseSpecificationImpl$HamcrestAssertionClosure.validate(ResponseSpecificationImpl.groovy:493) at io.restassured.internal.ResponseSpecificationImpl$HamcrestAssertionClosure$validate$1.call(Unknown Source) at io.restassured.internal.ResponseSpecificationImpl.validateResponseIfRequired(ResponseSpecificationImpl.groovy:674) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAcc ``` The root cause for this issue is upgrade of io.micrometer:micrometer-registry-graphite from 1.3.16 to 1.5.14 as transitive dependency of spring-boot. In the newer version of micrometer-registry-graphite, there is addition of native Graphite tag support and making it as default behaviour from 1.4+ version. Now the legacy hierarchical tag names need to be explicitly enabled by disabling native graphite support. https://github.com/micrometer-metrics/micrometer/releases/tag/v1.4.0 micrometer-metrics/micrometer#1806 Based on the discussions [here](micrometer-metrics/micrometer-docs#123) spring-boot has also exposed the property to disable the native graphite tag support, and implemented in 2.3.x [here](spring-projects/spring-boot#20834). To fix this issue, explicitly specifying this property `management.metrics.export.graphite.graphite-tags-enabled=false` in test environment and in kayenta config.

…g-boot to 2.3.12 (#909) While upgrading the spring-boot by transitively upgrading the orca version in kayenta and enforcing the dependencies, the kayenta-integration-tests:GraphiteStandaloneCanaryAnalysisTest failed with below error: ``` java.lang.AssertionError: 1 expectation failed. JSON path executionStatus doesn't match. Expected: is "SUCCEEDED" Actual: TERMINAL at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490) at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72) at org.codehaus.groovy.reflection.CachedConstructor.doConstructorInvoke(CachedConstructor.java:59) at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrap.callConstructor(ConstructorSite.java:84) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:277) at io.restassured.internal.ResponseSpecificationImpl$HamcrestAssertionClosure.validate(ResponseSpecificationImpl.groovy:493) at io.restassured.internal.ResponseSpecificationImpl$HamcrestAssertionClosure$validate$1.call(Unknown Source) at io.restassured.internal.ResponseSpecificationImpl.validateResponseIfRequired(ResponseSpecificationImpl.groovy:674) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAcc ``` The root cause for this issue is upgrade of io.micrometer:micrometer-registry-graphite from 1.3.16 to 1.5.14 as transitive dependency of spring-boot. In the newer version of micrometer-registry-graphite, there is addition of native Graphite tag support and making it as default behaviour from 1.4+ version. Now the legacy hierarchical tag names need to be explicitly enabled by disabling native graphite support. https://github.com/micrometer-metrics/micrometer/releases/tag/v1.4.0 micrometer-metrics/micrometer#1806 Based on the discussions [here](micrometer-metrics/micrometer-docs#123) spring-boot has also exposed the property to disable the native graphite tag support, and implemented in 2.3.x [here](spring-projects/spring-boot#20834). To fix this issue, explicitly specifying this property `management.metrics.export.graphite.graphite-tags-enabled=false` in test environment and in kayenta config.

WIP graphite tags

864c2da

fitzoh commented Jan 18, 2020

View reviewed changes

...micrometer-registry-graphite/src/main/java/io/micrometer/graphite/GraphiteTagNameMapper.java Outdated Show resolved Hide resolved

fitzoh commented Jan 18, 2020

View reviewed changes

...ometer-registry-graphite/src/test/java/io/micrometer/graphite/GraphiteTagNameMapperTest.java Outdated Show resolved Hide resolved

fitzoh commented Jan 18, 2020

View reviewed changes

...micrometer-registry-graphite/src/main/java/io/micrometer/graphite/GraphiteTagNameMapper.java Outdated Show resolved Hide resolved

fitzoh commented Jan 18, 2020

View reviewed changes

...ometer-registry-graphite/src/test/java/io/micrometer/graphite/GraphiteTagNameMapperTest.java Outdated Show resolved Hide resolved

jkschneider reviewed Jan 25, 2020

View reviewed changes

jkschneider changed the title ~~WIP Graphite tag support~~ Graphite tag support Jan 25, 2020

fitzoh added 3 commits January 25, 2020 16:09

Update copyright year

5b5a6ae

Remove tagsAsPrefix option for GraphiteTagNameMapper.

a1bb940

Add test suite for Tag based Graphite naming

612566a

fitzoh commented Jan 25, 2020

View reviewed changes

...ometer-registry-graphite/src/test/java/io/micrometer/graphite/GraphiteTagNameMapperTest.java Outdated Show resolved Hide resolved

fitzoh commented Jan 25, 2020

View reviewed changes

...ometer-registry-graphite/src/test/java/io/micrometer/graphite/GraphiteTagNameMapperTest.java Outdated Show resolved Hide resolved

fitzoh added 6 commits January 25, 2020 17:53

Rename GraphiteTagNameMapper -> GraphiteDimensionalNameMapper

0b11415

GraphiteNamingConvention -> GrahpiteHierarchicalNamingConvention

179d0dc

Initial copy-paste of new graphite dimensional naming convention clas…

c936e97

…s + tests

Update tests + implementation for GraphiteDimensionalNamingConvention

ad899ae

Simplify GraphiteDimensionalNameMapperTest (most of the tests are now…

7a24ed9

… handled by the naming convention tests)

Handle empty tag key/values

1a9cd7a

jkschneider suggested changes Jan 26, 2020

View reviewed changes

...istry-graphite/src/main/java/io/micrometer/graphite/GraphiteDimensionalNamingConvention.java Outdated Show resolved Hide resolved

fitzoh added 3 commits January 25, 2020 19:57

checkstyle

2e08e9c

Enable Graphite tags by default

3e7e6a9

Javadoc + minor tweaks

116dda3

jkschneider merged commit f2f559f into micrometer-metrics:master Jan 27, 2020

jkschneider added this to the 1.4.0 milestone Jan 27, 2020

shakuzen modified the milestone: 1.4.0 Jan 28, 2020

shakuzen added registry: graphite A Graphite Registry related issue release notes Noteworthy change to call out in the release notes labels Jan 28, 2020

snicoll mentioned this pull request Apr 4, 2020

Expose Graphite's graphiteTagsEnabled property spring-projects/spring-boot#20834

Closed

snicoll mentioned this pull request Apr 4, 2020

Graphite documentation does not document native tags support micrometer-metrics/micrometer-docs#123

Closed

fitzoh deleted the graphite-tags branch April 18, 2020 21:16

fitzoh mentioned this pull request Apr 18, 2020

Update default value logic for graphite tags #2007

Merged

fitzoh mentioned this pull request May 9, 2020

Support sending Graphite metric attributes as tags dropwizard/metrics#1576

Merged

j-sandy mentioned this pull request Sep 12, 2022

fix(test): Issue with kayenta-integration-tests while upgrading spring-boot to 2.3.12 spinnaker/kayenta#909

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Graphite tag support #1806

Graphite tag support #1806

Uh oh!

fitzoh commented Jan 18, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkschneider commented Jan 27, 2020

Uh oh!

davide-baldo commented May 7, 2020

Uh oh!

shakuzen commented May 8, 2020

Uh oh!

Uh oh!

Graphite tag support #1806

Graphite tag support #1806

Uh oh!

Conversation

fitzoh commented Jan 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkschneider commented Jan 27, 2020

Uh oh!

davide-baldo commented May 7, 2020

Uh oh!

shakuzen commented May 8, 2020

Uh oh!

Uh oh!

fitzoh commented Jan 18, 2020 •

edited

Loading