Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ public class TestService {
"strongly-typed",
"tags",
"server-side-polling",
"fdv1-fallback"
"fdv1-fallback",
"instance-id"
};

static final Gson gson = new GsonBuilder().serializeNulls().create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.launchdarkly.sdk.server.subsystems.HttpConfiguration;
import com.launchdarkly.sdk.server.subsystems.LoggingConfiguration;

import java.util.UUID;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;

Expand Down Expand Up @@ -37,7 +38,7 @@ private ClientContextImpl(
) {
super(baseContext.getSdkKey(), baseContext.getApplicationInfo(), baseContext.getHttp(),
baseContext.getLogging(), baseContext.isOffline(), baseContext.getServiceEndpoints(),
baseContext.getThreadPriority(), baseContext.getWrapperInfo());
baseContext.getThreadPriority(), baseContext.getWrapperInfo(), baseContext.getInstanceId());
this.sharedExecutor = sharedExecutor;
this.diagnosticStore = diagnosticStore;
this.dataSourceUpdateSink = null;
Expand Down Expand Up @@ -79,22 +80,29 @@ static ClientContextImpl fromConfig(
LDConfig config,
ScheduledExecutorService sharedExecutor
) {
// Generate the instance ID once and thread it through every ClientContext we build for this
// LDClient. Subsystems built from any of these contexts will all observe the same value.
String instanceId = UUID.randomUUID().toString();

ClientContext minimalContext = new ClientContext(sdkKey, config.applicationInfo, null,
null, config.offline, config.serviceEndpoints, config.threadPriority, config.wrapperInfo);
null, config.offline, config.serviceEndpoints, config.threadPriority, config.wrapperInfo,
instanceId);
LoggingConfiguration loggingConfig = config.logging.build(minimalContext);

ClientContext contextWithLogging = new ClientContext(sdkKey, config.applicationInfo, null,
loggingConfig, config.offline, config.serviceEndpoints, config.threadPriority, config.wrapperInfo);
loggingConfig, config.offline, config.serviceEndpoints, config.threadPriority,
config.wrapperInfo, instanceId);
HttpConfiguration httpConfig = config.http.build(contextWithLogging);

if (httpConfig.getProxy() != null) {
contextWithLogging.getBaseLogger().info("Using proxy: {} {} authentication.",
httpConfig.getProxy(),
httpConfig.getProxyAuthentication() == null ? "without" : "with");
}

ClientContext contextWithHttpAndLogging = new ClientContext(sdkKey, config.applicationInfo, httpConfig,
loggingConfig, config.offline, config.serviceEndpoints, config.threadPriority, config.wrapperInfo);

ClientContext contextWithHttpAndLogging = new ClientContext(sdkKey, config.applicationInfo,
httpConfig, loggingConfig, config.offline, config.serviceEndpoints, config.threadPriority,
config.wrapperInfo, instanceId);

// Create a diagnostic store only if diagnostics are enabled. Diagnostics are enabled as long as 1. the
// opt-out property was not set in the config, and 2. we are using the standard event processor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,15 @@ private Result transformResult(com.launchdarkly.sdk.server.subsystems.EventSende
}

static final class HttpConfigurationBuilderImpl extends HttpConfigurationBuilder {
/**
* HTTP header used to identify this SDK instance for the purpose of estimating
* server-connection-minutes when polling. It contains a v4 UUID that is generated once per SDK
* instance and remains constant for the lifetime of the client.
*
* <p>See: sdk-specs / SCMP-server-connection-minutes-polling.
*/
static final String INSTANCE_ID_HEADER = "X-LaunchDarkly-Instance-Id";

@Override
public HttpConfiguration build(ClientContext clientContext) {
LDLogger logger = clientContext.getBaseLogger();
Expand Down Expand Up @@ -340,6 +349,16 @@ else if (wrapperName != null) {
headers.put("X-LaunchDarkly-Wrapper", wrapperId);
}

// The instance ID originates on ClientContext (generated once when LDClient is constructed)
// so every subsystem built from the same context observes a consistent value for the
// lifetime of the SDK instance.
String instanceId = clientContext.getInstanceId();
if (instanceId != null && !instanceId.isEmpty()) {
headers.put(INSTANCE_ID_HEADER, instanceId);
}

// For consistency with other SDKs, custom headers are allowed to overwrite headers such as
// User-Agent and Authorization.
if (!customHeaders.isEmpty()) {
headers.putAll(customHeaders);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import com.launchdarkly.sdk.server.interfaces.ServiceEndpoints;
import com.launchdarkly.sdk.server.interfaces.WrapperInfo;

import java.util.UUID;

/**
* Context information provided by the {@link com.launchdarkly.sdk.server.LDClient} when creating components.
* <p>
Expand All @@ -31,11 +33,18 @@ public class ClientContext {
private final boolean offline;
private final ServiceEndpoints serviceEndpoints;
private final int threadPriority;
private final String instanceId;
private WrapperInfo wrapperInfo;

/**
* Constructor that sets all properties. All should be non-null.
*
* Constructor that sets all properties including an explicit instance ID. All should be
* non-null.
*
* <p>The instance ID is sent on every outbound request in the {@code X-LaunchDarkly-Instance-Id}
* header. It must be generated once per LDClient and remain stable for the client's lifetime.
* The eight-argument constructor auto-generates a v4 UUID for callers that do not need to
* supply their own value.
*
* @param sdkKey the SDK key
* @param applicationInfo application metadata properties from
* {@link Builder#applicationInfo(com.launchdarkly.sdk.server.integrations.ApplicationInfoBuilder)}
Expand All @@ -46,6 +55,7 @@ public class ClientContext {
* {@link Builder#serviceEndpoints(com.launchdarkly.sdk.server.integrations.ServiceEndpointsBuilder)}
* @param threadPriority worker thread priority from {@link Builder#threadPriority(int)}
* @param wrapperInfo wrapper configuration from {@link Builder#wrapper(com.launchdarkly.sdk.server.integrations.WrapperInfoBuilder)}
* @param instanceId per-LDClient identifier for the {@code X-LaunchDarkly-Instance-Id} header
*/
public ClientContext(
String sdkKey,
Expand All @@ -55,7 +65,8 @@ public ClientContext(
boolean offline,
ServiceEndpoints serviceEndpoints,
int threadPriority,
WrapperInfo wrapperInfo
WrapperInfo wrapperInfo,
String instanceId
) {
this.sdkKey = sdkKey;
this.applicationInfo = applicationInfo;
Expand All @@ -65,19 +76,51 @@ public ClientContext(
this.serviceEndpoints = serviceEndpoints;
this.threadPriority = threadPriority;
this.wrapperInfo = wrapperInfo;

this.instanceId = instanceId;

this.baseLogger = logging == null ? LDLogger.none() :
LDLogger.withAdapter(logging.getLogAdapter(), logging.getBaseLoggerName());
}


/**
* Constructor that sets all properties. All should be non-null. Auto-generates a v4 UUID for
* the instance ID; use the nine-argument constructor if you need to thread an existing value
* through (for example, when copying a context for an in-flight LDClient).
*
* @param sdkKey the SDK key
* @param applicationInfo application metadata properties from
* {@link Builder#applicationInfo(com.launchdarkly.sdk.server.integrations.ApplicationInfoBuilder)}
* @param http HTTP configuration properties from {@link Builder#http(ComponentConfigurer)}
* @param logging logging configuration properties from {@link Builder#logging(ComponentConfigurer)}
* @param offline true if the SDK should be entirely offline
* @param serviceEndpoints service endpoint URI properties from
* {@link Builder#serviceEndpoints(com.launchdarkly.sdk.server.integrations.ServiceEndpointsBuilder)}
* @param threadPriority worker thread priority from {@link Builder#threadPriority(int)}
* @param wrapperInfo wrapper configuration from {@link Builder#wrapper(com.launchdarkly.sdk.server.integrations.WrapperInfoBuilder)}
*/
public ClientContext(
String sdkKey,
ApplicationInfo applicationInfo,
HttpConfiguration http,
LoggingConfiguration logging,
boolean offline,
ServiceEndpoints serviceEndpoints,
int threadPriority,
WrapperInfo wrapperInfo
) {
this(sdkKey, applicationInfo, http, logging, offline, serviceEndpoints, threadPriority,
wrapperInfo, UUID.randomUUID().toString());
}

/**
* Copy constructor.
*
*
* @param copyFrom the instance to copy from
*/
protected ClientContext(ClientContext copyFrom) {
this(copyFrom.sdkKey, copyFrom.applicationInfo, copyFrom.http, copyFrom.logging,
copyFrom.offline, copyFrom.serviceEndpoints, copyFrom.threadPriority, copyFrom.wrapperInfo);
copyFrom.offline, copyFrom.serviceEndpoints, copyFrom.threadPriority, copyFrom.wrapperInfo,
copyFrom.instanceId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test constructor produces inconsistent instance ID state

Low Severity

The single-arg ClientContext(String sdkKey) constructor calls defaultHttp(sdkKey), which internally creates a temporary ClientContext (auto-generating UUID-A) and builds an HttpConfiguration embedding UUID-A in headers. Then the main constructor delegates to the 8-arg version, which auto-generates a different UUID-B for this.instanceId. This means getInstanceId() returns UUID-B while getHttp().getDefaultHeaders()["X-LaunchDarkly-Instance-Id"] contains UUID-A — an internal inconsistency that could confuse future test authors.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 30cf64b. Configure here.

}

/**
Expand Down Expand Up @@ -199,13 +242,24 @@ public ServiceEndpoints getServiceEndpoints() {
/**
* Returns the worker thread priority that is set by
* {@link Builder#threadPriority(int)}.
*
*
* @return the thread priority
*/
public int getThreadPriority() {
return threadPriority;
}

/**
* Returns the per-LDClient instance identifier sent in the {@code X-LaunchDarkly-Instance-Id}
* header on outbound requests. The value is generated once when the {@link ClientContext} is
* constructed and is stable for the client's lifetime.
*
* @return the instance ID
*/
public String getInstanceId() {
return instanceId;
}

/**
* Returns the wrapper information.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,14 @@ public void ignoreEmptyFilter() throws Exception {
private void verifyHeaders(RequestInfo req) {
HttpConfiguration httpConfig = clientContext(sdkKey, LDConfig.DEFAULT).getHttp();
for (Map.Entry<String, String> kv: httpConfig.getDefaultHeaders()) {
// X-LaunchDarkly-Instance-Id is a per-HttpConfiguration random UUID, so the value generated
// here won't match the one used by the requestor in the test. We only verify that *some*
// instance ID header is present on the request; per-builder uniqueness is covered in
// HttpConfigurationBuilderTest.
if (kv.getKey().equals("X-LaunchDarkly-Instance-Id")) {
assertNotNull(req.getHeader(kv.getKey()));
continue;
}
assertThat(req.getHeader(kv.getKey()), equalTo(kv.getValue()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import java.net.URI;
import java.time.Duration;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;

import static com.launchdarkly.sdk.server.TestComponents.clientContext;
Expand Down Expand Up @@ -198,7 +200,14 @@ public void testHttpDefaults() {
assertEquals(defaults.getSocketTimeout(), hc.getSocketTimeout());
assertNull(hc.getSslSocketFactory());
assertNull(hc.getTrustManager());
assertEquals(ImmutableMap.copyOf(defaults.getDefaultHeaders()), ImmutableMap.copyOf(hc.getDefaultHeaders()));
// The X-LaunchDarkly-Instance-Id header is a fresh UUID per HttpConfiguration, so it will
// differ between the two configurations; compare the remaining headers and verify the
// instance-id header is present on both.
Map<String, String> defaultHeaders = new HashMap<>(ImmutableMap.copyOf(defaults.getDefaultHeaders()));
Map<String, String> hcHeaders = new HashMap<>(ImmutableMap.copyOf(hc.getDefaultHeaders()));
assertNotNull(defaultHeaders.remove("X-LaunchDarkly-Instance-Id"));
assertNotNull(hcHeaders.remove("X-LaunchDarkly-Instance-Id"));
assertEquals(defaultHeaders, hcHeaders);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,13 @@ public void verifyStreamRequestProperties() throws Exception {
assertThat(req.getPath(), equalTo("/all"));

for (Map.Entry<String, String> kv: httpConfig.getDefaultHeaders()) {
// X-LaunchDarkly-Instance-Id is a per-HttpConfiguration random UUID and the
// configuration here is a fresh build, distinct from the one used by the stream
// processor; only assert presence.
if (kv.getKey().equals("X-LaunchDarkly-Instance-Id")) {
assertNotNull(req.getHeader(kv.getKey()));
continue;
}
assertThat(req.getHeader(kv.getKey()), equalTo(kv.getValue()));
}
assertThat(req.getHeader("Accept"), equalTo("text/event-stream"));
Expand Down
Loading
Loading