Skip to content

Commit

Permalink
Fix DGS Micrometer timer configuration
Browse files Browse the repository at this point in the history
A recent update broke the documented autotime-related properties that live under
"management.metrics.dgs-graphql.autotime", since it renamed the property on the
DgsGraphQLMetricsProperties class from "autoTime" to "autoTimeProperties". Fix the
naming to restore the old config prefix, and add a test case that covers it.

Additionally, move the AutoTimer / PropertiesAutoTimer out of DgsGraphQLMetricsProperties,
and make it a dependency of DgsGraphQLMetricsInstrumentation and the other classes that
depend on it.
  • Loading branch information
kilink committed Jan 23, 2024
1 parent 32d65fd commit 4ce0571
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import io.micrometer.core.instrument.Tag
import io.micrometer.core.instrument.Timer
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import org.springframework.boot.actuate.metrics.AutoTimer
import java.util.Optional
import java.util.concurrent.CompletableFuture
import java.util.concurrent.CompletionStage
Expand All @@ -45,7 +46,8 @@ class DgsGraphQLMetricsInstrumentation(
private val tagsProvider: DgsGraphQLMetricsTagsProvider,
private val properties: DgsGraphQLMetricsProperties,
private val limitedTagMetricResolver: LimitedTagMetricResolver,
private val optQuerySignatureRepository: Optional<QuerySignatureRepository> = Optional.empty()
private val optQuerySignatureRepository: Optional<QuerySignatureRepository> = Optional.empty(),
private val autoTimer: AutoTimer
) : SimplePerformantInstrumentation() {

companion object {
Expand Down Expand Up @@ -80,11 +82,7 @@ class DgsGraphQLMetricsInstrumentation(
addAll(state.tags())
}

state.stopTimer(
properties.autotime
.builder(GqlMetric.QUERY.key)
.tags(tags)
)
state.stopTimer(autoTimer.builder(GqlMetric.QUERY.key).tags(tags))
}
}

Expand Down Expand Up @@ -217,8 +215,7 @@ class DgsGraphQLMetricsInstrumentation(
val recordedTags = baseTags + tagsProvider.getFieldFetchTags(state, parameters, error)

timerSampler.stop(
properties
.autotime
autoTimer
.builder(GqlMetric.RESOLVER.key)
.tags(recordedTags)
.register(registry)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
package com.netflix.graphql.dgs.metrics.micrometer

import org.springframework.boot.actuate.autoconfigure.metrics.AutoTimeProperties
import org.springframework.boot.actuate.autoconfigure.metrics.PropertiesAutoTimer
import org.springframework.boot.context.properties.ConfigurationProperties
import org.springframework.boot.context.properties.NestedConfigurationProperty
import org.springframework.boot.context.properties.bind.DefaultValue

@ConfigurationProperties("management.metrics.dgs-graphql")
data class DgsGraphQLMetricsProperties(
/** Auto-timed queries settings. */
var autotimeProperties: AutoTimeProperties = AutoTimeProperties(),
/** Auto-timer. */
@NestedConfigurationProperty
var autotime: PropertiesAutoTimer = PropertiesAutoTimer(autotimeProperties),
var autotime: AutoTimeProperties = AutoTimeProperties(),
/** Settings that can be used to limit some of the tag metrics used by DGS. */
@NestedConfigurationProperty
var tags: TagsProperties = TagsProperties(),
/** Settings to selectively enable/disable gql timers.*/
@NestedConfigurationProperty
var resolver: ResolverMetricProperties = ResolverMetricProperties(),
@NestedConfigurationProperty
var query: QueryMetricProperties = QueryMetricProperties()

) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry
import org.springframework.beans.factory.ObjectProvider
import org.springframework.boot.actuate.autoconfigure.metrics.CompositeMeterRegistryAutoConfiguration
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration
import org.springframework.boot.actuate.autoconfigure.metrics.PropertiesAutoTimer
import org.springframework.boot.autoconfigure.AutoConfiguration
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
Expand Down Expand Up @@ -53,12 +54,13 @@ open class DgsGraphQLMicrometerAutoConfiguration {
optQuerySignatureRepository: Optional<QuerySignatureRepository>
): DgsGraphQLMetricsInstrumentation {
return DgsGraphQLMetricsInstrumentation(
dgsSchemaProvider,
meterRegistrySupplier,
tagsProvider,
properties,
limitedTagMetricResolver,
optQuerySignatureRepository
schemaProvider = dgsSchemaProvider,
registrySupplier = meterRegistrySupplier,
tagsProvider = tagsProvider,
properties = properties,
limitedTagMetricResolver = limitedTagMetricResolver,
optQuerySignatureRepository = optQuerySignatureRepository,
autoTimer = PropertiesAutoTimer(properties.autotime)
)
}

Expand Down Expand Up @@ -125,9 +127,9 @@ open class DgsGraphQLMicrometerAutoConfiguration {
optCacheManager: Optional<CacheManager>
): QuerySignatureRepository {
return CacheableQuerySignatureRepository(
properties,
meterRegistrySupplier,
optCacheManager
autoTimer = PropertiesAutoTimer(properties.autotime),
meterRegistrySupplier = meterRegistrySupplier,
optionalCacheManager = optCacheManager
)
}

Expand All @@ -137,7 +139,7 @@ open class DgsGraphQLMicrometerAutoConfiguration {
properties: DgsGraphQLMetricsProperties,
meterRegistrySupplier: DgsMeterRegistrySupplier
): QuerySignatureRepository {
return SimpleQuerySignatureRepository(properties, meterRegistrySupplier)
return SimpleQuerySignatureRepository(PropertiesAutoTimer(properties.autotime), meterRegistrySupplier)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ package com.netflix.graphql.dgs.metrics.micrometer.utils

import com.github.benmanes.caffeine.cache.Caffeine
import com.netflix.graphql.dgs.Internal
import com.netflix.graphql.dgs.metrics.micrometer.DgsGraphQLMetricsProperties
import com.netflix.graphql.dgs.metrics.micrometer.DgsMeterRegistrySupplier
import graphql.language.Document
import io.micrometer.core.instrument.binder.cache.CaffeineCacheMetrics
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import org.springframework.boot.actuate.metrics.AutoTimer
import org.springframework.cache.Cache
import org.springframework.cache.CacheManager
import org.springframework.cache.caffeine.CaffeineCacheManager
Expand Down Expand Up @@ -53,10 +53,10 @@ import java.util.*
*/
@Internal
open class CacheableQuerySignatureRepository(
properties: DgsGraphQLMetricsProperties,
autoTimer: AutoTimer,
meterRegistrySupplier: DgsMeterRegistrySupplier,
private val optionalCacheManager: Optional<CacheManager>
) : SimpleQuerySignatureRepository(properties, meterRegistrySupplier) {
) : SimpleQuerySignatureRepository(autoTimer, meterRegistrySupplier) {

private lateinit var cache: Cache

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.netflix.graphql.dgs.metrics.micrometer.utils
import com.netflix.graphql.dgs.Internal
import com.netflix.graphql.dgs.metrics.DgsMetrics.CommonTags
import com.netflix.graphql.dgs.metrics.DgsMetrics.InternalMetric
import com.netflix.graphql.dgs.metrics.micrometer.DgsGraphQLMetricsProperties
import com.netflix.graphql.dgs.metrics.micrometer.DgsMeterRegistrySupplier
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters
import graphql.language.Document
Expand All @@ -29,6 +28,7 @@ import io.micrometer.core.instrument.Timer
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import org.springframework.beans.factory.InitializingBean
import org.springframework.boot.actuate.metrics.AutoTimer
import java.util.*

/**
Expand All @@ -38,7 +38,7 @@ import java.util.*
*/
@Internal
open class SimpleQuerySignatureRepository(
private val properties: DgsGraphQLMetricsProperties,
private val autoTimer: AutoTimer,
private val meterRegistrySupplier: DgsMeterRegistrySupplier
) : QuerySignatureRepository, InitializingBean {

Expand Down Expand Up @@ -77,7 +77,7 @@ open class SimpleQuerySignatureRepository(
tags += CommonTags.JAVA_CLASS.tags(this)
tags += CommonTags.JAVA_CLASS_METHOD.tags("get")
timerSample.stop(
properties.autotime
autoTimer
.builder(InternalMetric.TIMED_METHOD.key)
.tags(tags)
.register(meterRegistry)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ internal class DgsGraphQLMetricsPropertiesTest {

assertThat(props).isNotNull
assertThat(props.autotime.isEnabled).isTrue
assertThat(props.autotimeProperties.percentiles).isNull()
assertThat(props.autotimeProperties.isPercentilesHistogram).isFalse
assertThat(props.autotime.percentiles).isNull()
assertThat(props.autotime.isPercentilesHistogram).isFalse

assertThat(props.tags).isNotNull
assertThat(props.tags.limiter.kind).isEqualTo(DgsGraphQLMetricsProperties.CardinalityLimiterKind.FIRST)
assertThat(props.tags.limiter.limit).isEqualTo(100)
assertThat(props.tags.complexity.enabled).isEqualTo(true)
assertThat(props.tags.complexity.enabled).isTrue()

assertThat(props.resolver.enabled).isTrue()
assertThat(props.query.enabled).isTrue()
Expand Down Expand Up @@ -72,7 +72,7 @@ internal class DgsGraphQLMetricsPropertiesTest {
).run { ctx ->
val props = ctx.getBean(DgsGraphQLMetricsProperties::class.java)

assertThat(props.tags.complexity.enabled).isEqualTo(false)
assertThat(props.tags.complexity.enabled).isFalse()
}
}

Expand All @@ -84,7 +84,7 @@ internal class DgsGraphQLMetricsPropertiesTest {
).run { ctx ->
val props = ctx.getBean(DgsGraphQLMetricsProperties::class.java)

assertThat(props.resolver.enabled).isEqualTo(false)
assertThat(props.resolver.enabled).isFalse()
}
}

Expand All @@ -96,11 +96,32 @@ internal class DgsGraphQLMetricsPropertiesTest {
).run { ctx ->
val props = ctx.getBean(DgsGraphQLMetricsProperties::class.java)

assertThat(props.query.enabled).isEqualTo(false)
assertThat(props.query.enabled).isFalse()
}
}

@Configuration
@Test
fun `can override autotime configuration`() {
contextRunner.withPropertyValues(
"management.metrics.dgs-graphql.autotime.percentiles-histogram=true",
"management.metrics.dgs-graphql.autotime.percentiles=0.50,0.95,0.99"
)
.run { ctx ->
val props = ctx.getBean(DgsGraphQLMetricsProperties::class.java)
assertThat(props.autotime.isPercentilesHistogram).isTrue()
assertThat(props.autotime.percentiles).isEqualTo(doubleArrayOf(0.50, 0.95, 0.99))
}

contextRunner.withPropertyValues(
"management.metrics.dgs-graphql.autotime.enabled=false"
)
.run { ctx ->
val props = ctx.getBean(DgsGraphQLMetricsProperties::class.java)
assertThat(props.autotime.isEnabled).isFalse()
}
}

@Configuration(proxyBeanMethods = false)
@EnableConfigurationProperties(DgsGraphQLMetricsProperties::class)
open class TestConfiguration
}

0 comments on commit 4ce0571

Please sign in to comment.