-
Notifications
You must be signed in to change notification settings - Fork 945
Add auto configuration for spring scheduling instrumentation using aop #12438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
90 changes: 90 additions & 0 deletions
90
...oconfigure/internal/instrumentation/scheduling/SpringSchedulingInstrumentationAspect.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.scheduling; | ||
|
||
import io.opentelemetry.api.OpenTelemetry; | ||
import io.opentelemetry.api.common.AttributeKey; | ||
import io.opentelemetry.context.Context; | ||
import io.opentelemetry.context.Scope; | ||
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; | ||
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesGetter; | ||
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor; | ||
import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; | ||
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; | ||
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; | ||
import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; | ||
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; | ||
import org.aspectj.lang.ProceedingJoinPoint; | ||
import org.aspectj.lang.annotation.Around; | ||
import org.aspectj.lang.annotation.Aspect; | ||
import org.aspectj.lang.annotation.Pointcut; | ||
import org.aspectj.lang.reflect.MethodSignature; | ||
import org.springframework.aop.framework.AopProxyUtils; | ||
|
||
/** | ||
* Spring Scheduling instrumentation aop. | ||
* | ||
* <p>This aspect would intercept all methods annotated with {@link | ||
* org.springframework.scheduling.annotation.Scheduled} and {@link | ||
* org.springframework.scheduling.annotation.Schedules}. | ||
* | ||
* <p>Normally this would cover most of the Spring Scheduling use cases, but if you register jobs | ||
* programmatically such as {@link | ||
* org.springframework.scheduling.config.ScheduledTaskRegistrar#addCronTask}, this aspect would not | ||
* cover them. You may use {@link io.opentelemetry.instrumentation.annotations.WithSpan} to trace | ||
* these jobs manually. | ||
*/ | ||
@Aspect | ||
final class SpringSchedulingInstrumentationAspect { | ||
public static final String INSTRUMENTATION_NAME = "io.opentelemetry.spring-scheduling-3.1"; | ||
private final Instrumenter<ClassAndMethod, Object> instrumenter; | ||
|
||
public SpringSchedulingInstrumentationAspect( | ||
OpenTelemetry openTelemetry, ConfigProperties configProperties) { | ||
CodeAttributesGetter<ClassAndMethod> codedAttributesGetter = | ||
ClassAndMethod.codeAttributesGetter(); | ||
InstrumenterBuilder<ClassAndMethod, Object> builder = | ||
Instrumenter.builder( | ||
openTelemetry, | ||
INSTRUMENTATION_NAME, | ||
CodeSpanNameExtractor.create(codedAttributesGetter)) | ||
.addAttributesExtractor(CodeAttributesExtractor.create(codedAttributesGetter)); | ||
if (configProperties.getBoolean( | ||
"otel.instrumentation.spring-scheduling.experimental-span-attributes", false)) { | ||
builder.addAttributesExtractor( | ||
AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "spring_scheduling")); | ||
} | ||
instrumenter = builder.buildInstrumenter(); | ||
} | ||
|
||
@Pointcut( | ||
"@annotation(org.springframework.scheduling.annotation.Scheduled)" | ||
+ "|| @annotation(org.springframework.scheduling.annotation.Schedules)") | ||
public void pointcut() { | ||
// ignored | ||
} | ||
|
||
@Around("pointcut()") | ||
public Object execution(ProceedingJoinPoint joinPoint) throws Throwable { | ||
Context parent = Context.current(); | ||
ClassAndMethod request = | ||
ClassAndMethod.create( | ||
AopProxyUtils.ultimateTargetClass(joinPoint.getTarget()), | ||
((MethodSignature) joinPoint.getSignature()).getMethod().getName()); | ||
if (!instrumenter.shouldStart(parent, request)) { | ||
return joinPoint.proceed(); | ||
} | ||
Context context = instrumenter.start(parent, request); | ||
try (Scope ignored = context.makeCurrent()) { | ||
Object object = joinPoint.proceed(); | ||
instrumenter.end(context, request, object, null); | ||
return object; | ||
} catch (Throwable t) { | ||
instrumenter.end(context, request, null, t); | ||
throw t; | ||
} | ||
} | ||
} |
34 changes: 34 additions & 0 deletions
34
...internal/instrumentation/scheduling/SpringSchedulingInstrumentationAutoConfiguration.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.scheduling; | ||
|
||
import io.opentelemetry.api.OpenTelemetry; | ||
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.ConditionalOnEnabledInstrumentation; | ||
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; | ||
import org.aspectj.lang.annotation.Aspect; | ||
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; | ||
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; | ||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
import org.springframework.scheduling.annotation.Scheduled; | ||
|
||
/** | ||
* Configures an aspect for tracing. | ||
* | ||
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change | ||
* at any time. | ||
*/ | ||
@ConditionalOnBean(OpenTelemetry.class) | ||
@ConditionalOnEnabledInstrumentation(module = "spring-scheduling") | ||
@ConditionalOnClass({Scheduled.class, Aspect.class}) | ||
@Configuration | ||
class SpringSchedulingInstrumentationAutoConfiguration { | ||
@Bean | ||
SpringSchedulingInstrumentationAspect springSchedulingInstrumentationAspect( | ||
OpenTelemetry openTelemetry, ConfigProperties configProperties) { | ||
return new SpringSchedulingInstrumentationAspect(openTelemetry, configProperties); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
210 changes: 210 additions & 0 deletions
210
...utoconfigure/internal/instrumentation/scheduling/SchedulingInstrumentationAspectTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,210 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.scheduling; | ||
|
||
import static io.opentelemetry.api.trace.SpanKind.INTERNAL; | ||
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; | ||
import static io.opentelemetry.sdk.testing.assertj.TracesAssert.assertThat; | ||
import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_FUNCTION; | ||
import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_NAMESPACE; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
||
import io.opentelemetry.api.OpenTelemetry; | ||
import io.opentelemetry.api.trace.Span; | ||
import io.opentelemetry.api.trace.Tracer; | ||
import io.opentelemetry.context.Context; | ||
import io.opentelemetry.context.Scope; | ||
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; | ||
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; | ||
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; | ||
import io.opentelemetry.sdk.trace.data.SpanData; | ||
import io.opentelemetry.sdk.trace.data.StatusData; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
import org.springframework.aop.aspectj.annotation.AspectJProxyFactory; | ||
import org.springframework.scheduling.annotation.Scheduled; | ||
import org.springframework.scheduling.annotation.Schedules; | ||
|
||
class SchedulingInstrumentationAspectTest { | ||
|
||
@RegisterExtension | ||
static final LibraryInstrumentationExtension testing = LibraryInstrumentationExtension.create(); | ||
|
||
private InstrumentationSchedulingTester schedulingTester; | ||
private String unproxiedTesterSimpleClassName; | ||
private String unproxiedTesterClassName; | ||
|
||
SpringSchedulingInstrumentationAspect newAspect( | ||
OpenTelemetry openTelemetry, ConfigProperties configProperties) { | ||
return new SpringSchedulingInstrumentationAspect(openTelemetry, configProperties); | ||
} | ||
|
||
@BeforeEach | ||
void setup() { | ||
InstrumentationSchedulingTester unproxiedTester = | ||
new InstrumentationSchedulingTester(testing.getOpenTelemetry()); | ||
unproxiedTesterSimpleClassName = unproxiedTester.getClass().getSimpleName(); | ||
unproxiedTesterClassName = unproxiedTester.getClass().getName(); | ||
|
||
AspectJProxyFactory factory = new AspectJProxyFactory(); | ||
factory.setTarget(unproxiedTester); | ||
|
||
SpringSchedulingInstrumentationAspect aspect = | ||
newAspect( | ||
testing.getOpenTelemetry(), | ||
DefaultConfigProperties.createFromMap(Collections.emptyMap())); | ||
factory.addAspect(aspect); | ||
|
||
schedulingTester = factory.getProxy(); | ||
} | ||
|
||
@Test | ||
@DisplayName("when method is annotated with @Scheduled should start a new span.") | ||
void scheduled() { | ||
// when | ||
schedulingTester.testScheduled(); | ||
|
||
// then | ||
List<List<SpanData>> traces = testing.waitForTraces(1); | ||
assertThat(traces) | ||
.hasTracesSatisfyingExactly( | ||
trace -> | ||
trace.hasSpansSatisfyingExactly( | ||
span -> | ||
span.hasName(unproxiedTesterSimpleClassName + ".testScheduled") | ||
.hasKind(INTERNAL) | ||
.hasAttributesSatisfyingExactly( | ||
equalTo(CODE_NAMESPACE, unproxiedTesterClassName), | ||
equalTo(CODE_FUNCTION, "testScheduled")))); | ||
} | ||
|
||
@Test | ||
@DisplayName("when method is annotated with multiple @Scheduled should start a new span.") | ||
void multiScheduled() { | ||
// when | ||
schedulingTester.testMultiScheduled(); | ||
|
||
// then | ||
List<List<SpanData>> traces = testing.waitForTraces(1); | ||
assertThat(traces) | ||
.hasTracesSatisfyingExactly( | ||
trace -> | ||
trace.hasSpansSatisfyingExactly( | ||
span -> | ||
span.hasName(unproxiedTesterSimpleClassName + ".testMultiScheduled") | ||
.hasKind(INTERNAL) | ||
.hasAttributesSatisfyingExactly( | ||
equalTo(CODE_NAMESPACE, unproxiedTesterClassName), | ||
equalTo(CODE_FUNCTION, "testMultiScheduled")))); | ||
} | ||
|
||
@Test | ||
@DisplayName("when method is annotated with @Schedules should start a new span.") | ||
void schedules() { | ||
// when | ||
schedulingTester.testSchedules(); | ||
|
||
// then | ||
List<List<SpanData>> traces = testing.waitForTraces(1); | ||
assertThat(traces) | ||
.hasTracesSatisfyingExactly( | ||
trace -> | ||
trace.hasSpansSatisfyingExactly( | ||
span -> | ||
span.hasName(unproxiedTesterSimpleClassName + ".testSchedules") | ||
.hasKind(INTERNAL) | ||
.hasAttributesSatisfyingExactly( | ||
equalTo(CODE_NAMESPACE, unproxiedTesterClassName), | ||
equalTo(CODE_FUNCTION, "testSchedules")))); | ||
} | ||
|
||
@Test | ||
@DisplayName( | ||
"when method is annotated with @Scheduled and it starts nested span, spans should be nested.") | ||
void nestedSpanInScheduled() { | ||
// when | ||
schedulingTester.testNestedSpan(); | ||
|
||
// then | ||
List<List<SpanData>> traces = testing.waitForTraces(1); | ||
assertThat(traces) | ||
.hasTracesSatisfyingExactly( | ||
trace -> | ||
trace.hasSpansSatisfyingExactly( | ||
span -> | ||
span.hasName(unproxiedTesterSimpleClassName + ".testNestedSpan") | ||
.hasKind(INTERNAL) | ||
.hasAttributesSatisfyingExactly( | ||
equalTo(CODE_NAMESPACE, unproxiedTesterClassName), | ||
equalTo(CODE_FUNCTION, "testNestedSpan")), | ||
nestedSpan -> | ||
nestedSpan.hasParent(trace.getSpan(0)).hasKind(INTERNAL).hasName("test"))); | ||
} | ||
|
||
@Test | ||
@DisplayName( | ||
"when method is annotated with @Scheduled AND an exception is thrown span should record the exception") | ||
void scheduledError() { | ||
assertThatThrownBy(() -> schedulingTester.testScheduledWithException()) | ||
.isInstanceOf(Exception.class); | ||
|
||
List<List<SpanData>> traces = testing.waitForTraces(1); | ||
assertThat(traces) | ||
.hasTracesSatisfyingExactly( | ||
trace -> | ||
trace.hasSpansSatisfyingExactly( | ||
span -> | ||
span.hasName(unproxiedTesterSimpleClassName + ".testScheduledWithException") | ||
.hasKind(INTERNAL) | ||
.hasStatus(StatusData.error()) | ||
.hasAttributesSatisfyingExactly( | ||
equalTo(CODE_NAMESPACE, unproxiedTesterClassName), | ||
equalTo(CODE_FUNCTION, "testScheduledWithException")))); | ||
} | ||
|
||
static class InstrumentationSchedulingTester { | ||
private final OpenTelemetry openTelemetry; | ||
|
||
InstrumentationSchedulingTester(OpenTelemetry openTelemetry) { | ||
this.openTelemetry = openTelemetry; | ||
} | ||
|
||
@Scheduled(fixedRate = 1L) | ||
public void testScheduled() { | ||
// no-op | ||
} | ||
|
||
@Scheduled(fixedRate = 1L) | ||
@Scheduled(fixedRate = 2L) | ||
public void testMultiScheduled() { | ||
// no-op | ||
} | ||
|
||
@Schedules({@Scheduled(fixedRate = 1L), @Scheduled(fixedRate = 2L)}) | ||
public void testSchedules() { | ||
// no-op | ||
} | ||
|
||
@Scheduled(fixedRate = 1L) | ||
public void testNestedSpan() { | ||
Context current = Context.current(); | ||
Tracer tracer = openTelemetry.getTracer("test"); | ||
try (Scope ignored = current.makeCurrent()) { | ||
Span span = tracer.spanBuilder("test").startSpan(); | ||
span.end(); | ||
} | ||
} | ||
|
||
@Scheduled(fixedRate = 1L) | ||
public void testScheduledWithException() { | ||
throw new IllegalStateException("something went wrong"); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you trying to enable the AspectJ support with Spring? In this case, you could use
@EnableAspectJAutoProxy
(https://docs.spring.io/spring-framework/reference/core/aop/ataspectj/aspectj-support.html)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, these codes are purposed for unit tests to test the functions of
SpringSchedulingInstrumentationAspect
. If we use@EnableAspectJAutoProxy
annotation, we must start theAnnotationConfigApplicationContext
which is too heavy for tests since we only need to test the function of Aop I think. UsingAspectJProxyFactory
to create proxy instance and let it parse and apply theSpringSchedulingInstrumentationAspect
will also meet our demands for tests.