Skip to content
5 changes: 5 additions & 0 deletions spring-cloud-config-client/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@
<artifactId>spring-boot-starter-actuator</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-restclient</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-aspectj</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class ConfigServerBootstrapper implements BootstrapRegistryInitializer {

private LoaderInterceptor loaderInterceptor;

static ConfigServerBootstrapper create() {
public static ConfigServerBootstrapper create() {
Copy link
Copy Markdown
Member

@jonatan-ivanov jonatan-ivanov May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be public?
It seems the only new place where this is used is ObservationConfigServerBootstrapper which is in the same package and it might be package-private as well as ConfigServerBootstrapper.

return new ConfigServerBootstrapper();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,14 @@ public List<ConfigServerConfigDataResource> resolveProfileSpecific(
.registerSingleton("configDataConfigClientProperties",
event.getBootstrapContext().get(ConfigClientProperties.class)));

bootstrapContext.registerIfAbsent(ConfigClientRequestTemplateFactory.class,
context -> new ConfigClientRequestTemplateFactory(log, context.get(ConfigClientProperties.class)));
bootstrapContext.registerIfAbsent(ConfigClientRequestTemplateFactory.class, context -> {
ConfigClientProperties props = context.get(ConfigClientProperties.class);
if (ClassUtils
.isPresent("org.springframework.boot.restclient.observation.ObservationRestTemplateCustomizer", null)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point we are too early in the lifecycle here to check the presence of the bean instead of the class, right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct. At this point we are in the bootstrap phase, so the ApplicationContext
does not exist yet and we cannot check for bean presence. Checking for class presence via
ClassUtils.isPresent is the only viable option here.
Is there anything we are missing?

return ObservationConfigClientRequestTemplateFactory.createWithObservation(context, log, props);
}
return new ConfigClientRequestTemplateFactory(log, props);
});

bootstrapContext.registerIfAbsent(RestTemplate.class, context -> {
ConfigClientRequestTemplateFactory factory = context.get(ConfigClientRequestTemplateFactory.class);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2026-present the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.cloud.config.client;

import io.micrometer.observation.ObservationRegistry;
import org.apache.commons.logging.Log;

import org.springframework.boot.bootstrap.BootstrapContext;
import org.springframework.boot.restclient.observation.ObservationRestTemplateCustomizer;
import org.springframework.http.client.observation.DefaultClientRequestObservationConvention;
import org.springframework.web.client.RestTemplate;

public class ObservationConfigClientRequestTemplateFactory extends ConfigClientRequestTemplateFactory {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public (as well as its ctor)?
I'm thinking if ConfigClientRequestTemplateFactory being public is enough?


private final ObservationRestTemplateCustomizer observationRestTemplateCustomizer;

public ObservationConfigClientRequestTemplateFactory(Log log, ConfigClientProperties properties,
ObservationRegistry observationRegistry) {
super(log, properties);
this.observationRestTemplateCustomizer = observationRegistry != ObservationRegistry.NOOP
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use !observationRegistry.isNoop() instead (it's more flexible).

? new ObservationRestTemplateCustomizer(observationRegistry,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we/should we reuse the bean that Boot creates (see RestTemplateObservationAutoConfiguration)?

I mean if the user disables RestTemplateObservationAutoConfiguration, should the observation config for this RestTemplate disabled too?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use the addCloseListener approach, this would be partially handled — if the user
disables RestTemplateObservationAutoConfiguration, the ObservationRestTemplateCustomizer
bean would not exist in the ApplicationContext and the RestTemplate would not be
re-instrumented after bootstrap.

However, the RestTemplate would still be instrumented during the bootstrap phase since
the ObservationRegistry is created there before the ApplicationContext exists.

To fully respect the user's intent, we would need to check during bootstrap whether the
user has disabled the observation support. One possible approach would be to read a
property (e.g. spring.cloud.config.client.observation.enabled) via the Binder during
bootstrap, since it is available at that point.

Does this approach make sense, or is there a better way to handle this?

new DefaultClientRequestObservationConvention())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one reason why we should not create a new ObservationRestTemplateCustomizer, Boot does something different: it lets users to customize the convention. Even if we duplicate exactly what Boot does, how can we keep this in-sync?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. Using a hardcoded DefaultClientRequestObservationConvention prevents
users from customizing the convention and creates a sync problem with what Boot does.

We thought about an approach to solve both issues — the convention and the registry sync:
using addCloseListener from the BootstrapContext to re-instrument the RestTemplate when
the ApplicationContext is ready, at which point both the ObservationRegistry (with handlers
already registered by ObservationRegistryPostProcessor) and the user's custom
ClientRequestObservationConvention bean would be available.

The limitation of this approach is that observations from the initial bootstrap call
(the first fetch from Config Server) would not be captured, since the ApplicationContext
does not exist yet at that point. Observations from refreshes would be captured correctly.

Given that the issue mentions retries as a key use case:

  1. Is it acceptable to miss observations from the initial bootstrap call?
  2. Is re-instrumenting via addCloseListener the right approach here?
  3. Are there any flaws in this reasoning or anything we might have misunderstood?

: null;
}

@Override
public RestTemplate create() {
RestTemplate template = super.create();
if (observationRestTemplateCustomizer != null) {
observationRestTemplateCustomizer.customize(template);
}
return template;
}

static ConfigClientRequestTemplateFactory createWithObservation(BootstrapContext context, Log log,
ConfigClientProperties props) {
ObservationRegistry registry = context.getOrElse(ObservationRegistry.class, ObservationRegistry.NOOP);
return new ObservationConfigClientRequestTemplateFactory(log, props, registry);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2026-present the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.cloud.config.client;

import io.micrometer.observation.ObservationRegistry;

import org.springframework.boot.bootstrap.BootstrapRegistry;

public class ObservationConfigServerBootstrapper extends ConfigServerBootstrapper {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public (and its ctor)?


private ObservationRegistry observationRegistry;

public static ObservationConfigServerBootstrapper create() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed (we already have a ctor with the same empty params)?

return new ObservationConfigServerBootstrapper();
}

public ObservationConfigServerBootstrapper withObservationRegistry(ObservationRegistry observationRegistry) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to enable this by default without the user need to do this?
(from your PR description):

SpringApplication app = new SpringApplication(MyApp.class);
app.addBootstrapRegistryInitializer(
    new ConfigServerBootstrapper()
        .withObservationRegistry(myObservationRegistry)
);
app.run(args);

Also, I have two questions in connection with this:

  • Didn't you want to write: new ObservationConfigServerBootstrapper() instead of new ConfigServerBootstrapper()?
  • Where exactly should users get the myObservationRegistry? (Given that it is created by Boot later and we don't want users to end-up with multiple registries.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, the PR description should use new ObservationConfigServerBootstrapper()
instead of new ConfigServerBootstrapper() — that was a mistake in the description.

Regarding enabling this by default without the user needing to configure it manually —
we are analyzing an approach using addCloseListener to re-instrument the RestTemplate
when the ApplicationContext is ready, which would make this automatic. We will share
more details in the other comment.

Regarding where users should get the ObservationRegistry — this is exactly the problem
we are trying to solve. The ObservationRegistry is created by Boot in the ApplicationContext,
which does not exist yet during bootstrap. The addCloseListener approach would solve this
by accessing the Boot-created ObservationRegistry directly from the ApplicationContext,
avoiding multiple registries. Does this approach make sense, or is there anything we are missing? We will share more details in the other comment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding where users should get the ObservationRegistry — this is exactly the problem we are trying to solve.

You mean this PR should solve it or you have an idea to solve this? (I'm asking because I think the current changes PR do not solve the issue.)

The addCloseListener approach would solve this
by accessing the Boot-created ObservationRegistry directly from the ApplicationContext,
avoiding multiple registries. Does this approach make sense, or is there anything we are missing?

I don't know, can you tell me more about the the addCloseListener approach or can you share some code I can look at?

Copy link
Copy Markdown
Author

@LuccasAps LuccasAps May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to use addCloseListener from the BootstrapContext to re-instrument the RestTemplate
when the ApplicationContext is ready, at which point the Boot-created ObservationRegistry
(with handlers already registered) and the user's custom ClientRequestObservationConvention
bean would be available.

Here is a rough idea of how this would work:

bootstrapContext.addCloseListener(event -> {
    ApplicationContext applicationContext = event.getApplicationContext();
    if (applicationContext.containsBean(ObservationRestTemplateCustomizer.class.getName())) {
        ObservationRestTemplateCustomizer customizer = applicationContext
            .getBean(ObservationRestTemplateCustomizer.class);
        RestTemplate restTemplate = event.getBootstrapContext().get(RestTemplate.class);
        customizer.customize(restTemplate);
    }
});

The limitation of this approach is that the initial bootstrap call to the Config Server
would not be captured since the ApplicationContext does not exist yet at that point.
Observations from refreshes would be captured correctly.

Does this approach make sense? Is there a better way to handle this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The limitation of this approach is that the initial bootstrap call to the Config Server would not be captured since the ApplicationContext does not exist yet at that point.

I guess that's ok, maybe we can add more logs (in a different issue/PR) if needed to help people troubleshooting things.

Does this approach make sense? Is there a better way to handle this?

I think it makes sense. Or let me say I can imagine a situation where this would work but the last time I needed to interact with the bootstrap context was many years ago. :) Have you tried this?

@ryanjbaxter What do you think?

this.observationRegistry = observationRegistry;
return this;
}

@Override
public void initialize(BootstrapRegistry registry) {
super.initialize(registry);
if (observationRegistry != null) {
registry.register(ObservationRegistry.class, BootstrapRegistry.InstanceSupplier.of(observationRegistry));
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright 2026-present the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.cloud.config.client;

import io.micrometer.observation.ObservationRegistry;
import org.apache.commons.logging.LogFactory;
import org.junit.jupiter.api.Test;

import org.springframework.web.client.RestTemplate;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Luccas Asaphe
*
*/
public class ConfigClientRequestTemplateFactoryTests {

@Test
void shouldInstrumentRestTemplateWhenObservationRegistryProvided() {
Comment thread
LuccasAps marked this conversation as resolved.
Outdated
// 1. set up the factory
ConfigClientProperties properties = new ConfigClientProperties();
ObservationRegistry registry = ObservationRegistry.create();
ObservationConfigClientRequestTemplateFactory factory = new ObservationConfigClientRequestTemplateFactory(
LogFactory.getLog(getClass()), properties, registry);

// 2. create the RestTemplate
RestTemplate restTemplate = factory.create();

// 3. verify the observation registry
assertThat(restTemplate.getObservationRegistry()).isEqualTo(registry);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be much better to verify if RestTemplate is indeed instrumented by:

  • Using ObservationRegistry
  • Calling RestTemplate
  • Verifying if Observations are created (see micrometer-observation-test)

}

@Test
void shouldNotInstrumentRestTemplateWhenObservationRegistryNotProvided() {
// 1. set up the factory
ConfigClientProperties properties = new ConfigClientProperties();
ConfigClientRequestTemplateFactory factory = new ConfigClientRequestTemplateFactory(
LogFactory.getLog(getClass()), properties);

// 2. create the RestTemplate
RestTemplate restTemplate = factory.create();

// 3. verify the observation registry
assertThat(restTemplate.getObservationRegistry()).isEqualTo(ObservationRegistry.NOOP);
}

@Test
void shouldNotInstrumentRestTemplateWhenObservationRegistryIsNoop() {
// 1. set up the factory
ConfigClientProperties properties = new ConfigClientProperties();
ObservationRegistry registry = ObservationRegistry.NOOP;
Comment thread
LuccasAps marked this conversation as resolved.
Outdated
ObservationConfigClientRequestTemplateFactory factory = new ObservationConfigClientRequestTemplateFactory(
LogFactory.getLog(getClass()), properties, registry);

// 2. create the RestTemplate
RestTemplate restTemplate = factory.create();

// 3. verify the observation registry
assertThat(restTemplate.getObservationRegistry()).isEqualTo(ObservationRegistry.NOOP);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.util.Optional;

import io.micrometer.observation.ObservationRegistry;
import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
Expand Down Expand Up @@ -91,6 +92,34 @@ void customizableRestTemplate() {
}
}

@Test
void customizableObservationRegistry() {
ConfigurableApplicationContext context = null;
try {
ObservationRegistry registry = ObservationRegistry.create();
context = new SpringApplicationBuilder(TestConfig.class)
.addBootstrapRegistryInitializer(
ObservationConfigServerBootstrapper.create().withObservationRegistry(registry))
.addBootstrapRegistryInitializer(reg -> reg.addCloseListener(event -> {
BootstrapContext bootstrapContext = event.getBootstrapContext();
ConfigurableListableBeanFactory beanFactory = event.getApplicationContext().getBeanFactory();

RestTemplate restTemplate = bootstrapContext.get(RestTemplate.class);
beanFactory.registerSingleton("holder", new RestTemplateHolder(restTemplate));
}))
.run("--spring.config.import=optional:configserver:");

RestTemplateHolder holder = context.getBean(RestTemplateHolder.class);
assertThat(holder).isNotNull();
assertThat(holder.restTemplate.getObservationRegistry()).isEqualTo(registry);
}
finally {
if (context != null) {
context.close();
}
}
}

CustomRestTemplate restTemplate(BootstrapContext context) {
ConfigClientProperties properties = context.get(ConfigClientProperties.class);
String custom = context.get(Binder.class).bind("custom.prop", String.class).orElse("default-custom-prop");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright 2026-present the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.cloud.config.client;

import org.junit.jupiter.api.Test;

import org.springframework.boot.SpringBootConfiguration;
import org.springframework.boot.WebApplicationType;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.builder.SpringApplicationBuilder;
import org.springframework.cloud.test.ClassPathExclusions;
import org.springframework.context.ConfigurableApplicationContext;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Luccas Asaphe
*
*/
@ClassPathExclusions({ "spring-boot-starter-actuator-*.jar", "spring-boot-restclient-*.jar" })
Comment thread
LuccasAps marked this conversation as resolved.
Outdated
public class ConfigServerConfigDataWithoutMicrometerTests {

@Test
void contextStartsWithoutMicrometer() {
Comment thread
LuccasAps marked this conversation as resolved.
try (ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class)
.web(WebApplicationType.NONE)
.run("--spring.config.import=optional:configserver:")) {
assertThat(context).isNotNull();
}
}

@SpringBootConfiguration
@EnableAutoConfiguration
static class TestConfig {

}

}
Loading