From c2e3fed8dcda5abc8a5f4ad4939344f8f44121c1 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 27 Jun 2023 22:11:05 +0200 Subject: [PATCH] Align observations of @Scheduled with OTel conventions This commit updates the `ScheduledTaskObservationDocumentation` to better align the contributed KeyValues with OpenTelemetry conventions for observations of code executions. Instead of a "target.type" key with the bean class simple name, this is now contributing the canonical class name of the bean under the "code.namespace" key. The "method.name" key is renamed to "code.function" and its values remain unchanged. Closes gh-30721 --- .../ROOT/pages/integration/observability.adoc | 4 +-- ...ultScheduledTaskObservationConvention.java | 18 +++++------ ...ScheduledTaskObservationDocumentation.java | 12 +++---- ...onBeanPostProcessorObservabilityTests.java | 32 +++++++++---------- ...heduledTaskObservationConventionTests.java | 5 +-- 5 files changed, 36 insertions(+), 35 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/integration/observability.adoc b/framework-docs/modules/ROOT/pages/integration/observability.adoc index 412bb54406c0..d077ccf134b0 100644 --- a/framework-docs/modules/ROOT/pages/integration/observability.adoc +++ b/framework-docs/modules/ROOT/pages/integration/observability.adoc @@ -101,10 +101,10 @@ By default, the following `KeyValues` are created: [cols="a,a"] |=== |Name | Description +|`code.function` _(required)_|Name of Java `Method` that is scheduled for execution. +|`code.namespace` _(required)_|Canonical name of the class of the bean instance that holds the scheduled method. |`exception` _(required)_|Name of the exception thrown during the execution, or `KeyValue#NONE_VALUE`} if no exception happened. -|`method.name` _(required)_|Name of Java `Method` that is scheduled for execution. |`outcome` _(required)_|Outcome of the method execution. Can be `"SUCCESS"`, `"ERROR"` or `"UNKNOWN"` (if for example the operation was cancelled during execution. -|`target.type` _(required)_|Simple class name of the bean instance that holds the scheduled method. |=== diff --git a/spring-context/src/main/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConvention.java b/spring-context/src/main/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConvention.java index 5ff2eb7641e8..99daaa42e4c3 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConvention.java +++ b/spring-context/src/main/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConvention.java @@ -53,7 +53,15 @@ public String getContextualName(ScheduledTaskObservationContext context) { @Override public KeyValues getLowCardinalityKeyValues(ScheduledTaskObservationContext context) { - return KeyValues.of(exception(context), methodName(context), outcome(context), targetType(context)); + return KeyValues.of(codeFunction(context), codeNamespace(context), exception(context), outcome(context)); + } + + protected KeyValue codeFunction(ScheduledTaskObservationContext context) { + return KeyValue.of(LowCardinalityKeyNames.CODE_FUNCTION, context.getMethod().getName()); + } + + protected KeyValue codeNamespace(ScheduledTaskObservationContext context) { + return KeyValue.of(LowCardinalityKeyNames.CODE_NAMESPACE, context.getTargetClass().getCanonicalName()); } protected KeyValue exception(ScheduledTaskObservationContext context) { @@ -63,10 +71,6 @@ protected KeyValue exception(ScheduledTaskObservationContext context) { return EXCEPTION_NONE; } - protected KeyValue methodName(ScheduledTaskObservationContext context) { - return KeyValue.of(LowCardinalityKeyNames.METHOD_NAME, context.getMethod().getName()); - } - protected KeyValue outcome(ScheduledTaskObservationContext context) { if (context.getError() != null) { return OUTCOME_ERROR; @@ -77,8 +81,4 @@ else if (!context.isComplete()) { return OUTCOME_SUCCESS; } - protected KeyValue targetType(ScheduledTaskObservationContext context) { - return KeyValue.of(LowCardinalityKeyNames.TARGET_TYPE, context.getTargetClass().getSimpleName()); - } - } diff --git a/spring-context/src/main/java/org/springframework/scheduling/config/ScheduledTaskObservationDocumentation.java b/spring-context/src/main/java/org/springframework/scheduling/config/ScheduledTaskObservationDocumentation.java index 0f466a8b1282..51850cf7d117 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/config/ScheduledTaskObservationDocumentation.java +++ b/spring-context/src/main/java/org/springframework/scheduling/config/ScheduledTaskObservationDocumentation.java @@ -56,22 +56,22 @@ public KeyName[] getHighCardinalityKeyNames() { public enum LowCardinalityKeyNames implements KeyName { /** - * {@link Class#getSimpleName() Simple name} of the target type that owns the scheduled method. + * Name of the method that is executed for the scheduled task. */ - TARGET_TYPE { + CODE_FUNCTION { @Override public String asString() { - return "target.type"; + return "code.function"; } }, /** - * Name of the method that is executed for the scheduled task. + * {@link Class#getCanonicalName() Canonical name} of the target type that owns the scheduled method. */ - METHOD_NAME { + CODE_NAMESPACE { @Override public String asString() { - return "method.name"; + return "code.namespace"; } }, diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorObservabilityTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorObservabilityTests.java index 5bb071b8f517..5578c7453148 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorObservabilityTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorObservabilityTests.java @@ -65,8 +65,8 @@ void shouldRecordSuccessObservationsForTasks() throws Exception { registerScheduledBean(FixedDelayBean.class); runScheduledTaskAndAwait(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS") - .hasLowCardinalityKeyValue("method.name", "fixedDelay") - .hasLowCardinalityKeyValue("target.type", "FixedDelayBean") + .hasLowCardinalityKeyValue("code.function", "fixedDelay") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".FixedDelayBean") .hasLowCardinalityKeyValue("exception", "none"); } @@ -75,8 +75,8 @@ void shouldRecordFailureObservationsForTasksThrowing() throws Exception { registerScheduledBean(FixedDelayErrorBean.class); runScheduledTaskAndAwait(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "ERROR") - .hasLowCardinalityKeyValue("method.name", "error") - .hasLowCardinalityKeyValue("target.type", "FixedDelayErrorBean") + .hasLowCardinalityKeyValue("code.function", "error") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".FixedDelayErrorBean") .hasLowCardinalityKeyValue("exception", "IllegalStateException"); } @@ -85,8 +85,8 @@ void shouldRecordSuccessObservationsForReactiveTasks() throws Exception { registerScheduledBean(FixedDelayReactiveBean.class); runScheduledTaskAndAwait(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS") - .hasLowCardinalityKeyValue("method.name", "fixedDelay") - .hasLowCardinalityKeyValue("target.type", "FixedDelayReactiveBean") + .hasLowCardinalityKeyValue("code.function", "fixedDelay") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".FixedDelayReactiveBean") .hasLowCardinalityKeyValue("exception", "none"); } @@ -95,8 +95,8 @@ void shouldRecordFailureObservationsForReactiveTasksThrowing() throws Exception registerScheduledBean(FixedDelayReactiveErrorBean.class); runScheduledTaskAndAwait(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "ERROR") - .hasLowCardinalityKeyValue("method.name", "error") - .hasLowCardinalityKeyValue("target.type", "FixedDelayReactiveErrorBean") + .hasLowCardinalityKeyValue("code.function", "error") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".FixedDelayReactiveErrorBean") .hasLowCardinalityKeyValue("exception", "IllegalStateException"); } @@ -108,8 +108,8 @@ void shouldRecordCancelledObservationsForTasks() throws Exception { context.getBean(TaskTester.class).await(); scheduledTask.cancel(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "UNKNOWN") - .hasLowCardinalityKeyValue("method.name", "cancelled") - .hasLowCardinalityKeyValue("target.type", "CancelledTaskBean") + .hasLowCardinalityKeyValue("code.function", "cancelled") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".CancelledTaskBean") .hasLowCardinalityKeyValue("exception", "none"); } @@ -121,8 +121,8 @@ void shouldRecordCancelledObservationsForReactiveTasks() throws Exception { context.getBean(TaskTester.class).await(); scheduledTask.cancel(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "UNKNOWN") - .hasLowCardinalityKeyValue("method.name", "cancelled") - .hasLowCardinalityKeyValue("target.type", "CancelledReactiveTaskBean") + .hasLowCardinalityKeyValue("code.function", "cancelled") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".CancelledReactiveTaskBean") .hasLowCardinalityKeyValue("exception", "none"); } @@ -131,8 +131,8 @@ void shouldHaveCurrentObservationInScope() throws Exception { registerScheduledBean(CurrentObservationBean.class); runScheduledTaskAndAwait(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS") - .hasLowCardinalityKeyValue("method.name", "hasCurrentObservation") - .hasLowCardinalityKeyValue("target.type", "CurrentObservationBean") + .hasLowCardinalityKeyValue("code.function", "hasCurrentObservation") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".CurrentObservationBean") .hasLowCardinalityKeyValue("exception", "none"); } @@ -141,8 +141,8 @@ void shouldHaveCurrentObservationInReactiveScope() throws Exception { registerScheduledBean(CurrentObservationReactiveBean.class); runScheduledTaskAndAwait(); assertThatTaskObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS") - .hasLowCardinalityKeyValue("method.name", "hasCurrentObservation") - .hasLowCardinalityKeyValue("target.type", "CurrentObservationReactiveBean") + .hasLowCardinalityKeyValue("code.function", "hasCurrentObservation") + .hasLowCardinalityKeyValue("code.namespace", getClass().getCanonicalName() + ".CurrentObservationReactiveBean") .hasLowCardinalityKeyValue("exception", "none"); } diff --git a/spring-context/src/test/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConventionTests.java b/spring-context/src/test/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConventionTests.java index bdf613380a7f..47da8360b974 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConventionTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/config/DefaultScheduledTaskObservationConventionTests.java @@ -58,13 +58,14 @@ void observationShouldHaveContextualNameForProxiedClass() { @Test void observationShouldHaveTargetType() { ScheduledTaskObservationContext context = new ScheduledTaskObservationContext(new BeanWithScheduledMethods(), taskMethod); - assertThat(convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("target.type", "BeanWithScheduledMethods")); + assertThat(convention.getLowCardinalityKeyValues(context)) + .contains(KeyValue.of("code.namespace", getClass().getCanonicalName() + ".BeanWithScheduledMethods")); } @Test void observationShouldHaveMethodName() { ScheduledTaskObservationContext context = new ScheduledTaskObservationContext(new BeanWithScheduledMethods(), taskMethod); - assertThat(convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("method.name", "process")); + assertThat(convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("code.function", "process")); } @Test