spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.26k stars 40.7k forks source link

Add support for the @SpanTag annotation #38662

Closed lkleeven closed 10 months ago

lkleeven commented 11 months ago

Spring Boot 3.2.0 came with a number of Observability improvements, supporting a number of Micrometer annotations.

@SpanTag is not mentioned here, and according to our tests, also not working out of the box. Is this done by design or would this be a bug?

To make things easier, I've made a demo project with a unit test that reproduces the issue. You can do so by opening the project and running the AopTracingTest or running ./mvnw clean verify. Next to the @SpanTag annotation it also verifies the working of other tracing annotations, which currently do work.

bclozel commented 11 months ago

Looking at #37640, it seems that the application needs to define a custom SpanTagAnnotationHandler bean. @jonatan-ivanov can you explain why this is needed? Should we improve the documentation to mention our support for those annotations?

lkleeven commented 11 months ago

Maybe good to mention. While using Spring Boot 3.1.0 we provided this for the users of our framework with the following autoconfiguration where we did not need to define any SpanTagAnnotationHandler, also not anywhere else:

/**
 * Creates the beans needed to be able to use AOP for the {@link io.micrometer.tracing.annotation.NewSpan},
 * {@link io.micrometer.tracing.annotation.ContinueSpan} and {@link io.micrometer.tracing.annotation.SpanTag} annotations
 */
@AutoConfiguration(after = AxleOpenTelemetryTracingAutoConfiguration.class)
@ConditionalOnEnabledTracing
@ConditionalOnProperty(value = AxleTracingProperties.AopProperties.PROP_ENABLED, havingValue = "true", matchIfMissing = true)
public class AxleTracingAopAutoConfiguration {

    @Bean
    @ConditionalOnMissingBean
    public NewSpanParser axleNewSpanParser() {
        return new DefaultNewSpanParser();
    }

    @Bean
    @ConditionalOnMissingBean
    public MethodInvocationProcessor axleMethodInvocationProcessor(
            NewSpanParser newSpanParser,
            Tracer tracer,
            BeanFactory beanFactory
    ) {
        return new ImperativeMethodInvocationProcessor(
                newSpanParser,
                tracer,
                beanFactory::getBean,
                beanFactory::getBean
        );
    }

    @Bean
    @ConditionalOnMissingBean
    public SpanAspect axleSpanAspect(MethodInvocationProcessor methodInvocationProcessor) {
        return new SpanAspect(methodInvocationProcessor);
    }
}
jonatan-ivanov commented 11 months ago

SpanTagAnnotationHandler should be able to resolve the key and the value in some way from the @SpanTag annotation and set them on the span but right now we did not add an opinion about what that behavior should be. There are multiple ways to do this and different users might have different needs. We can add a default behavior I'm not sure though what should it be. Fyi: Micrometer Tracing has an example that supports SpEL: https://micrometer.io/docs/tracing#_aspect_oriented_programming_starting_from_micrometer_tracing_1_1_0

Btw there is also @MeterTag which right now only works for @Timed.

marcingrzejszczak commented 11 months ago

In Spring Cloud Sleuth the default was SPel. Micrometer Tracing has no notion of SPel nor Spring so it can't ship this component. That should be coming from Boot I guess? :shrug:

mhalbritter commented 11 months ago

I have something in this branch. It auto-configures the SpanTagAnnotationHandler and implements a SpEL based ValueExpressionResolver.

marcingrzejszczak commented 11 months ago

I checked it out and it makes perfect sense @mhalbritter :+1:

mhalbritter commented 11 months ago

We decided to ship that as an enhancement in 3.3. If you want to have it for 3.2 already, please see the code in my branch. It's not that much:

@Bean
@ConditionalOnMissingBean
SpanTagAnnotationHandler spanTagAnnotationHandler(BeanFactory beanFactory) {
  ValueExpressionResolver valueExpressionResolver = new SpelTagValueExpressionResolver();
  return new SpanTagAnnotationHandler(beanFactory::getBean, (ignored) -> valueExpressionResolver);
}
private static class SpelTagValueExpressionResolver implements ValueExpressionResolver {

  @Override
  public String resolve(String expression, Object parameter) {
    try {
      SimpleEvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding().build();
      ExpressionParser expressionParser = new SpelExpressionParser();
      Expression expressionToEvaluate = expressionParser.parseExpression(expression);
      return expressionToEvaluate.getValue(context, parameter, String.class);
    } catch (Exception ex) {
      throw new IllegalStateException("Unable to evaluate SpEL expression '%s'".formatted(expression), ex);
    }
  }
}