Skip to content
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

ObservationFilter beans are not registered automatically #33968

Closed
edwardsre opened this issue Jan 24, 2023 · 4 comments
Closed

ObservationFilter beans are not registered automatically #33968

edwardsre opened this issue Jan 24, 2023 · 4 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@edwardsre
Copy link
Contributor

edwardsre commented Jan 24, 2023

The Spring Boot 3.0 Migration Guide indicates the following, but this does not appear to be the case.

You can contribute ObservationFilter beans to your application and Spring Boot will auto-configure them with the ObservationRegistry.

Steps to duplicate

I am using Spring Boot 3.0.2

Create an application with Spring Initializer with Spring Web, Lombok, and Actuator dependencies

Modify the main application class as follows

@SpringBootApplication
@RestController
@Slf4j
public class DemoApplication {

  public static void main(String[] args) {
    SpringApplication.run(DemoApplication.class, args);
  }

  @GetMapping("test")
  public String testEndpoint() {
    return "hello world";
  }

  @Bean
  public ObservationFilter customMethodFilter() {
    return context -> {
      log.info("Executing observation filter");
      if (context instanceof ServerRequestObservationContext observationContext) {
        context.addLowCardinalityKeyValue(KeyValue.of("custom.method", observationContext.getCarrier().getMethod()));
      }
      return context;
    };
  }
}

Run the application and perform an HTTP request to http://localhost:8080/test. Notice the log statement is not printed.

Add the following bean into the main application class. Based on the docs, this step shouldn't be necessary.

  @Bean
  public ObservationRegistryCustomizer<?> myCustomizer(ObservationFilter customMethodFilter) {
    return registry -> registry.observationConfig().observationFilter(customMethodFilter);
  }

Restart the application and perform the same HTTP request to http://localhost:8080/test. Notice the log statement is now printed indicating the filter is being registered and used.

It appears that ObservationRegistryConfigurer created by ObservationRegistryPostProcessor does not get injected with a ObjectProvider<ObservationFilter>, but all other aspects of the registry can be configured this way. Is this an omission, or is the documentation incorrect?

I also can't find any usages of ObservationRegistry.observationConfig().observationFilter(...) that would indicate auto configuration is registering ObservationFilter beans provided by the application as stated in the migration guide.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 24, 2023
@edwardsre
Copy link
Contributor Author

edwardsre commented Jan 24, 2023

I found the Spring Boot Docs to be more accurate for the behavior I am seeing. The migration guide should be updated to remove the false statement.

It would be great if ObservationFilter beans could be auto registered as well.

@mhalbritter
Copy link
Contributor

Thanks for letting us know! I've removed the ObservationFilter line from the migration guide. I think it's a good idea to auto-register ObservationFilters.

@mhalbritter mhalbritter added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 25, 2023
@mhalbritter mhalbritter added this to the 3.1.x milestone Jan 25, 2023
@mhalbritter mhalbritter changed the title ObservationFilter beans are not added to ObservationRegistry by auto-configuration Automatically register ObservationFilter Jan 25, 2023
@mhalbritter mhalbritter changed the title Automatically register ObservationFilter Automatically register ObservationFilters Jan 25, 2023
@wilkinsona
Copy link
Member

I wonder if we should consider this to be a bug of omission as we registered MeterFilter beans automatically in 2.x. Also, the statement in the migration guide must have come from somewhere. I wonder if we overlooked something in one of the many observability-related changes we made in 3.0. Flagging for a team meeting so that we can have a chat about what we'd like to do.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 25, 2023
@marcingrzejszczak
Copy link
Contributor

I'd vote for assuming that this is a bug of omission. We should be auto injecting ObservationFilters to an ObservationRegistry.

@philwebb philwebb added type: bug A general bug and removed type: enhancement A general enhancement for: team-meeting An issue we'd like to discuss as a team to make progress labels Jan 30, 2023
@philwebb philwebb modified the milestones: 3.1.x, 3.0.x Jan 30, 2023
@wilkinsona wilkinsona changed the title Automatically register ObservationFilters ObservationFilter beans are not registered automatically Feb 8, 2023
@wilkinsona wilkinsona self-assigned this Feb 8, 2023
@wilkinsona wilkinsona modified the milestones: 3.0.x, 3.0.3 Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants