spring-cloud/spring-cloud-sleuth

GrpcTracing bean cannot be injected at startup

dimi-nk opened this issue · 7 comments

Describe the bug

I've recently introduced distributed tracing into one of our projects with the use of spring sleuth. Brave defines a GrpcTracing class which can create a ClientInterceptor via #newClientInterceptor. GrpcTracing is provided by TraceGrpcAutoConfiguration.

Since LogNet/grpc-spring-boot-starter doesn't include a mechanism to define global ClientInterceptors (as it does with @GRpcGlobalInterceptor), I created a small piece of code that gathers all provided ClientInterceptors and attaches them to gRPC channels.

So, for example, I'd want to provide brave's TracingClientInterceptor doing the following:

@Bean
@ConditionalOnBean(GrpcTracing.class)
public ClientInterceptor grpcTracingClientInterceptor(GrpcTracing grpcTracing) {
  return grpcTracing.newClientInterceptor();
}

We have tracing disabled in the dev and test profiles, so GrpcTracing won't be available. Hence the conditional.

Then, a simplified version of my implementation would be the following (the problem is in the block above so I don't think going into too much details here matters):

@Bean
public FooServiceBlockingStub fooServiceStub(List<ClientInterceptor> clientInterceptors) {
  return xyz; // create channel and stub
}

Now... When I use @ConditionalOnBean(GrpcTracing.class), the ClientInterceptor is never created. Even when adding a breakpoint, it doesn't stop. Even weirder, when I use the condition TraceGrpcAutoConfiguration#grpcTracing doesn't execute either... RpcTracing gets created every time. If I remove the condition, both configurations load. If I inject Optional<GrpcTracing> both configurations work.

I tried @AutoConfigureAfter(TraceGrpcAutoConfiguration.class) on my configuration class but it didn't work either. IIRC, user configurations run after auto-configurations anyway 🤷‍♂️ . I'm struggling to see why GrpcTracing is not available during start-up. I don't have any other conditions on the configuration class or method. I checked all conditions on TraceGrpcAutoConfiguration and they are all met.

I'm at my wits' end. Do you have any idea why this doesn't work 👀 ?


In case it helps, this is the current version of my configuration class.

@Configuration
@AutoConfigureAfter(TraceGrpcAutoConfiguration.class)
public class TracingConfiguration {
  private final Map<String, String> commonTags;

  public TracingConfiguration(@Value("${git.commit.id:unknown}") String gitCommitId) {
    var commonTags = new HashMap<String, String>();

    commonTags.put("service.version", gitCommitId);
    EnvironmentMetadata.create()
        .toAttributes()
        .forEach((key, value) -> commonTags.put(key, valueToString(value)));

    this.commonTags = Map.copyOf(commonTags);
  }

  private static String valueToString(AttributeValue value) {
    return value.match(
        s -> s, Object::toString, Object::toString, Object::toString, Object::toString);
  }

  @Bean
  ClientInterceptor grpcClientInterceptor(Optional<GrpcTracing> grpcTracing) {
    return grpcTracing.map(GrpcTracing::newClientInterceptor).orElse(null);
  }

  @Bean
  SpanHandler commonTagsHandler() {
    return new SpanHandler() {
      @Override
      public boolean end(TraceContext traceContext, MutableSpan span, Cause cause) {
        if (cause == Cause.FINISHED) {
          commonTags.forEach(span::tag);
        }
        return true; // keep this span
      }
    };
  }

  /**
   * Create a propagation factory chain. Stackdriver always delegates first before resolving itself.
   * gRPC always tries to extract <code>grpc-trace-bin</code> otherwise delegates. B3 tries to
   * resolve.
   *
   * <p>Therefore, the chain of resolution is the first that manages to extract a context from
   * <code>grpc-trace-bin</code>, <code>B3</code>, and <code>X-Cloud-Trace-Context</code>.
   */
  @Bean
  FactoryBuilder customPropagationFactoryBuilder() {
    return BaggagePropagation.newFactoryBuilder(
        StackdriverTracePropagation.newFactory(new CustomPropagationFactory()));
  }

  private static class CustomPropagationFactory extends Factory {
    @Override
    public <K> Propagation<K> create(Propagation.KeyFactory<K> keyFactory) {
      return StringPropagationAdapter.create(this.get(), keyFactory);
    }

    @Override
    public Propagation<String> get() {
      return GrpcPropagation.create(B3Propagation.get());
    }
  }
}

The slightly more complicated version of the global client interceptor looks like the following. The only thing it does it take all provided client interceptors and pass them through include/exlude filters defined in properties.

@RequiredArgsConstructor
public class PropertyBasedGlobalClientInterceptorLoader implements GlobalClientInterceptorLoader {

  private final GrpcProperties properties;
  private final List<ClientInterceptor> injectedInterceptors;

  @Override
  public List<ClientInterceptor> globalInterceptors() {
    var interceptorFilters = properties.getClient().getGlobalInterceptors();

    if (interceptorFilters.getInclude().size() > 0) {
      return injectedInterceptors.stream()
          .filter(i -> interceptorFilters.getInclude().contains(i.getClass()))
          .collect(toList());
    } else if (interceptorFilters.getExclude().size() > 0) {
      return injectedInterceptors.stream()
          .filter(not(i -> interceptorFilters.getExclude().contains(i.getClass())))
          .collect(toList());
    } else {
      return injectedInterceptors;
    }
  }
}

and

@Configuration
@EnableConfigurationProperties(GrpcProperties.class)
public class GrpcChannelAutoConfiguration {

  @Bean
  public DomainResolver domainResolver() {
    return new DomainResolver();
  }

  @Bean
  @ConditionalOnMissingBean
  public GrpcChannelResolver channelResolver(
      DomainResolver domainResolver, Optional<GlobalClientInterceptorLoader> interceptorLoader) {
    return new DefaultGrpcChannelResolver(
        interceptorLoader.map(GlobalClientInterceptorLoader::globalInterceptors).orElse(List.of()),
        domainResolver);
  }

  @Bean
  @ConditionalOnMissingBean
  @ConditionalOnProperty(
      value = "acme.grpc.client.global-interceptors.enabled",
      havingValue = "true",
      matchIfMissing = true)
  public GlobalClientInterceptorLoader clientInterceptorLoader(
      GrpcProperties properties, Optional<List<ClientInterceptor>> injectedInterceptors) {
    return new PropertyBasedGlobalClientInterceptorLoader(
        properties, injectedInterceptors.orElse(List.of()));
  }
}

Can you upload this a sample that we can check out?

I think you should go with @AutoConfigureAfter(TraceGrpcAutoConfiguration.class) but you would have to register your own configuration as autoconfiguration in spring.factories (I don't see that file in your repo). Can you try adding this

org.springframework.boot.autoconfigure.EnableAutoConfiguration=\
your.package.YouCustomAutoConfiguration

in the src/main/resources/META-INF/spring.factories file and try again with the latest version of Sleuth?

@marcingrzejszczak hmm I didn't realise @AutoConfigureAfter doesn't work on non-autoconfiguration classes, this could be it. Is there some documentation on when to use @AutoConfigureBefore|After? I assumed spring calculated the object graph and determined the sequence of beans it needs to create.

@dimi-nk Yes, there are some docs around this topic:

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.