Skip to content

[BUG] Setting a proxy via OptimizelyFactory creates proxy-less clients #538

Closed
@ben-r

Description

@ben-r

Is there an existing issue for this?

  • I have searched the existing issues

SDK Version

4.0.0

Current Behavior

Currently the only way to set an dedicated proxy (disregarding environment settings) is via OptimizelyFactory.newDefaultInstance() (#330).

Optimizely instances created this way are able to use the proxy e.g. for fetching the data file (via cdn.optimizely.com), but both, AsyncEventHandler and the newly added ODPApiManager disregard the proxy setting by creating their own http clients further down the constructor chain.

public static Optimizely newDefaultInstance(String sdkKey, String fallback, String datafileAccessToken, CloseableHttpClient customHttpClient) {
        NotificationCenter notificationCenter = new NotificationCenter();
        OptimizelyHttpClient optimizelyHttpClient = new OptimizelyHttpClient(customHttpClient);
        HttpProjectConfigManager.Builder builder;
        builder = HttpProjectConfigManager.builder()
            .withDatafile(fallback)
            .withNotificationCenter(notificationCenter)
            .withOptimizelyHttpClient(customHttpClient == null ? null : optimizelyHttpClient) <-- uses correct client
            .withSdkKey(sdkKey);

        if (datafileAccessToken != null) {
            builder.withDatafileAccessToken(datafileAccessToken);
        }

        return newDefaultInstance(builder.build(), notificationCenter);
}
public static Optimizely newDefaultInstance(ProjectConfigManager configManager, NotificationCenter notificationCenter) {
        EventHandler eventHandler = AsyncEventHandler.builder().build(); <-- creates own client
        return newDefaultInstance(configManager, notificationCenter, eventHandler);
}
public static Optimizely newDefaultInstance(ProjectConfigManager configManager, NotificationCenter notificationCenter, EventHandler eventHandler) {
        if (notificationCenter == null) {
            notificationCenter = new NotificationCenter();
        }

        BatchEventProcessor eventProcessor = BatchEventProcessor.builder()
            .withEventHandler(eventHandler)
            .withNotificationCenter(notificationCenter)
            .build();

        ODPApiManager defaultODPApiManager = new DefaultODPApiManager(); <-- creates own clients
        ODPManager odpManager = ODPManager.builder()
            .withApiManager(defaultODPApiManager)
            .build();

        return Optimizely.builder()
            .withEventProcessor(eventProcessor)
            .withConfigManager(configManager)
            .withNotificationCenter(notificationCenter)
            .withODPManager(odpManager)
            .build();
}

Expected Behavior

Setting the proxy once should do it for every SDK-internal client.

Configuring the proxy is unnecessarily complicated at the moment. It should be part of the Optimizily.Builder API

Steps To Reproduce

  1. Create Optimizely instance via
import com.optimizely.ab.Optimizely;
import com.optimizely.ab.OptimizelyFactory;
import org.apache.http.HttpHost;
import org.apache.http.impl.client.HttpClients;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class OptimizelyConfig {

    @Bean
    ProxySettings externalProxy(
            @Value("${proxy.config.host:}") String host,
            @Value("${proxy.config.port:}") Integer port) {
        return new ProxySettings(host, port);
    }

    @Bean
    Optimizely createOptimizely(
            @Value("${optimizely.fullstack.sdk.key}") String optimizelySdkKey,
            @Qualifier("externalProxy") ProxySettings proxySettings) {

        var clientBuilder = HttpClients.custom();
        if (proxySettings.isEnabled()) {
            clientBuilder.setProxy(new HttpHost(proxySettings.getHost(), proxySettings.getPort()));
        }

        return OptimizelyFactory.newDefaultInstance(optimizelySdkKey, null, null, clientBuilder.build());
    }

    public record ProxySettings(String host, Integer port) {
        public boolean isEnabled() {
            return host != null && !host.isBlank() && port != null;
        }
    }
}
  1. Start local proxy, e.g. squid
  2. Fetch the data file via optimizely.getOptimizelyConfig().getDatafile() and verify it's not empty. You should see requests to cdn.optimizely.com in the proxy logs.
  3. Create an event via optimizely.track(). You should see that requests to logx.optimizely.com don't appear in the proxy logs

Java Version

21

Link

No response

Logs

org.apache.http.conn.ConnectTimeoutException: Connect to logx.optimizely.com:443 [logx.optimizely.com/34.111.140.246] failed: Connect timed out
at org.apache.http.impl.conn.DefaultHttpClientConnectionOperator.connect(DefaultHttpClientConnectionOperator.java:151)
at org.apache.http.impl.conn.PoolingHttpClientConnectionManager.connect(PoolingHttpClientConnectionManager.java:376)
at org.apache.http.impl.execchain.MainClientExec.establishRoute(MainClientExec.java:393)
at org.apache.http.impl.execchain.MainClientExec.execute(MainClientExec.java:236)
at org.apache.http.impl.execchain.ProtocolExec.execute(ProtocolExec.java:186)
at org.apache.http.impl.execchain.RetryExec.execute(RetryExec.java:89)
at org.apache.http.impl.execchain.RedirectExec.execute(RedirectExec.java:110)
at org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:185)
at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:72)
at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:221)
at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:165)
at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:140)
at com.optimizely.ab.OptimizelyHttpClient.execute(OptimizelyHttpClient.java:62)
at com.optimizely.ab.event.AsyncEventHandler$EventDispatcher.run(AsyncEventHandler.java:229)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.net.SocketTimeoutException: Connect timed out
at java.base/sun.nio.ch.NioSocketImpl.timedFinishConnect(NioSocketImpl.java:551)
at java.base/sun.nio.ch.NioSocketImpl.connect(NioSocketImpl.java:602)
at java.base/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:327)
at java.base/java.net.Socket.connect(Socket.java:633)
at org.apache.http.conn.ssl.SSLConnectionSocketFactory.connectSocket(SSLConnectionSocketFactory.java:368)
at org.apache.http.impl.conn.DefaultHttpClientConnectionOperator.connect(DefaultHttpClientConnectionOperator.java:142)
... 16 more

Severity

Blocking development

Workaround/Solution

No response

Recent Change

No response

Conflicts

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    acknowledgedThe issue has been acknowledged and being looked into. Further details will follow.bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions