Skip to content

Commit

Permalink
Remove stateful mutable field from Config (#341)
Browse files Browse the repository at this point in the history
* Remove stateful mutable field from Config

* non-empty attributes
  • Loading branch information
Mateusz Rzeszutek authored Aug 30, 2022
1 parent 1bf8277 commit dd28be3
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 111 deletions.
22 changes: 3 additions & 19 deletions splunk-otel-android/src/main/java/com/splunk/rum/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@

import android.util.Log;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.semconv.resource.attributes.ResourceAttributes;
import java.time.Duration;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Function;

Expand All @@ -43,7 +41,7 @@ public class Config {
private final boolean crashReportingEnabled;
private final boolean networkMonitorEnabled;
private final boolean anrDetectionEnabled;
private final AtomicReference<Attributes> globalAttributes = new AtomicReference<>();
private final Attributes globalAttributes;
private final Function<SpanExporter, SpanExporter> spanFilterExporterDecorator;
private final boolean slowRenderingDetectionEnabled;
private final Duration slowRenderingDetectionPollInterval;
Expand All @@ -58,7 +56,7 @@ private Config(Builder builder) {
this.debugEnabled = builder.debugEnabled;
this.applicationName = builder.applicationName;
this.crashReportingEnabled = builder.crashReportingEnabled;
this.globalAttributes.set(addDeploymentEnvironment(builder));
this.globalAttributes = addDeploymentEnvironment(builder);
this.networkMonitorEnabled = builder.networkMonitorEnabled;
this.anrDetectionEnabled = builder.anrDetectionEnabled;
this.slowRenderingDetectionPollInterval = builder.slowRenderingDetectionPollInterval;
Expand Down Expand Up @@ -118,7 +116,7 @@ public boolean isSlowRenderingDetectionEnabled() {
* instrumentation.
*/
public Attributes getGlobalAttributes() {
return globalAttributes.get();
return globalAttributes;
}

/** Is the network monitoring feature enabled or not. */
Expand Down Expand Up @@ -172,20 +170,6 @@ public static Builder builder() {
return new Builder();
}

void updateGlobalAttributes(Consumer<AttributesBuilder> updater) {
while (true) {
Attributes oldAttributes = globalAttributes.get();

AttributesBuilder builder = oldAttributes.toBuilder();
updater.accept(builder);
Attributes newAttributes = builder.build();

if (globalAttributes.compareAndSet(oldAttributes, newAttributes)) {
break;
}
}
}

SpanExporter decorateWithSpanFilter(SpanExporter exporter) {
return spanFilterExporterDecorator.apply(exporter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@

import android.os.Build;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.context.Context;
import io.opentelemetry.sdk.trace.ReadWriteSpan;
import io.opentelemetry.sdk.trace.ReadableSpan;
import io.opentelemetry.sdk.trace.SpanProcessor;
import java.util.function.Supplier;

class RumAttributeAppender implements SpanProcessor {
static final AttributeKey<String> APP_NAME_KEY = stringKey("app");
Expand All @@ -39,19 +41,22 @@ class RumAttributeAppender implements SpanProcessor {

static final AttributeKey<String> SPLUNK_OPERATION_KEY = stringKey("_splunk_operation");

private final Config config;
private final String applicationName;
private final Supplier<Attributes> globalAttributesSupplier;
private final SessionId sessionId;
private final String rumVersion;
private final VisibleScreenTracker visibleScreenTracker;
private final ConnectionUtil connectionUtil;

RumAttributeAppender(
Config config,
String applicationName,
Supplier<Attributes> globalAttributesSupplier,
SessionId sessionId,
String rumVersion,
VisibleScreenTracker visibleScreenTracker,
ConnectionUtil connectionUtil) {
this.config = config;
this.applicationName = applicationName;
this.globalAttributesSupplier = globalAttributesSupplier;
this.sessionId = sessionId;
this.rumVersion = rumVersion;
this.visibleScreenTracker = visibleScreenTracker;
Expand All @@ -64,7 +69,7 @@ public void onStart(Context parentContext, ReadWriteSpan span) {
// name on the wire.
span.setAttribute(SPLUNK_OPERATION_KEY, span.getName());

span.setAttribute(APP_NAME_KEY, config.getApplicationName());
span.setAttribute(APP_NAME_KEY, applicationName);
span.setAttribute(SESSION_ID_KEY, sessionId.getSessionId());
span.setAttribute(RUM_VERSION_KEY, rumVersion);

Expand All @@ -73,7 +78,7 @@ public void onStart(Context parentContext, ReadWriteSpan span) {
span.setAttribute(OS_NAME, "Android");
span.setAttribute(OS_TYPE, "linux");
span.setAttribute(OS_VERSION, Build.VERSION.RELEASE);
span.setAllAttributes(config.getGlobalAttributes());
span.setAllAttributes(globalAttributesSupplier.get());

String currentScreen = visibleScreenTracker.getCurrentlyVisibleScreen();
span.setAttribute(SplunkRum.SCREEN_NAME_KEY, currentScreen);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import android.util.Log;
import androidx.annotation.NonNull;
import com.splunk.android.rum.R;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.context.Context;
Expand All @@ -50,6 +51,7 @@
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
import java.util.logging.Level;
import zipkin2.reporter.Sender;
Expand All @@ -62,13 +64,15 @@ class RumInitializer {
static final int MAX_ATTRIBUTE_LENGTH = 256 * 128;

private final Config config;
private final AtomicReference<Attributes> globalAttributes;
private final Application application;
private final AppStartupTimer startupTimer;
private final List<RumInitializer.InitializationEvent> initializationEvents = new ArrayList<>();
private final AnchoredClock timingClock;

RumInitializer(Config config, Application application, AppStartupTimer startupTimer) {
this.config = config;
this.globalAttributes = new AtomicReference<>(config.getGlobalAttributes());
this.application = application;
this.startupTimer = startupTimer;
this.timingClock = startupTimer.startupClock;
Expand Down Expand Up @@ -163,7 +167,7 @@ SplunkRum initialize(Supplier<ConnectionUtil> connectionUtilSupplier, Looper mai

recordInitializationSpans(startTimeNanos, initializationEvents, tracer, config);

return new SplunkRum(openTelemetrySdk, sessionId, config);
return new SplunkRum(openTelemetrySdk, sessionId, globalAttributes);
}

private SlowRenderingDetector buildSlowRenderingDetector(Config config, Tracer tracer) {
Expand Down Expand Up @@ -281,7 +285,12 @@ private SdkTracerProvider buildTracerProvider(

RumAttributeAppender attributeAppender =
new RumAttributeAppender(
config, sessionId, rumVersion, visibleScreenTracker, connectionUtil);
config.getApplicationName(),
globalAttributes::get,
sessionId,
rumVersion,
visibleScreenTracker,
connectionUtil);
initializationEvents.add(
new RumInitializer.InitializationEvent(
"attributeAppenderInitialized", timingClock.now()));
Expand Down
22 changes: 18 additions & 4 deletions splunk-otel-android/src/main/java/com/splunk/rum/SplunkRum.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Supplier;
import okhttp3.Call;
Expand Down Expand Up @@ -76,17 +77,20 @@ public class SplunkRum {

private final SessionId sessionId;
private final OpenTelemetrySdk openTelemetrySdk;
private final Config config;
private final AtomicReference<Attributes> globalAttributes;

static {
Handler handler = new Handler(Looper.getMainLooper());
startupTimer.detectBackgroundStart(handler);
}

SplunkRum(OpenTelemetrySdk openTelemetrySdk, SessionId sessionId, Config config) {
SplunkRum(
OpenTelemetrySdk openTelemetrySdk,
SessionId sessionId,
AtomicReference<Attributes> globalAttributes) {
this.openTelemetrySdk = openTelemetrySdk;
this.sessionId = sessionId;
this.config = config;
this.globalAttributes = globalAttributes;
}

/** Create a new {@link Config.Builder} instance. */
Expand Down Expand Up @@ -321,7 +325,17 @@ public <T> SplunkRum setGlobalAttribute(AttributeKey<T> key, T value) {
* operating on a {@link AttributesBuilder} from the current set.
*/
public void updateGlobalAttributes(Consumer<AttributesBuilder> attributesUpdater) {
config.updateGlobalAttributes(attributesUpdater);
while (true) {
Attributes oldAttributes = globalAttributes.get();

AttributesBuilder builder = oldAttributes.toBuilder();
attributesUpdater.accept(builder);
Attributes newAttributes = builder.build();

if (globalAttributes.compareAndSet(oldAttributes, newAttributes)) {
break;
}
}
}

// for testing only
Expand Down
16 changes: 0 additions & 16 deletions splunk-otel-android/src/test/java/com/splunk/rum/ConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,22 +115,6 @@ public void creation_nullHandling() {
assertEquals(Attributes.empty(), config.getGlobalAttributes());
}

@Test
public void updateGlobalAttributes() {
Config config =
Config.builder()
.applicationName("appName")
.rumAccessToken("accessToken")
.realm("us0")
.globalAttributes(Attributes.of(stringKey("food"), "candy"))
.build();

config.updateGlobalAttributes(ab -> ab.put("drink", "lemonade"));
Attributes result = config.getGlobalAttributes();
assertEquals(
Attributes.of(stringKey("drink"), "lemonade", stringKey("food"), "candy"), result);
}

@Test
public void beaconOverridesRealm() {
Config config =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

public class RumAttributeAppenderTest {

private static final String APP_NAME = "appName";

private VisibleScreenTracker visibleScreenTracker;
private final ConnectionUtil connectionUtil = mock(ConnectionUtil.class);

Expand All @@ -49,11 +51,10 @@ public void setUp() {

@Test
public void interfaceMethods() {
Config config = mock(Config.class);
when(config.getGlobalAttributes()).thenReturn(Attributes.empty());
RumAttributeAppender rumAttributeAppender =
new RumAttributeAppender(
config,
APP_NAME,
Attributes::empty,
mock(SessionId.class),
"rumVersion",
visibleScreenTracker,
Expand All @@ -63,49 +64,10 @@ public void interfaceMethods() {
assertFalse(rumAttributeAppender.isEndRequired());
}

@Test
public void updateGlobalAttributes() {
Attributes initialAttributes =
Attributes.of(stringKey("cheese"), "Camembert", longKey("size"), 5L);

Config config =
Config.builder()
.globalAttributes(initialAttributes)
.realm("us0")
.rumAccessToken("123456")
.applicationName("appName")
.build();

RumAttributeAppender rumAttributeAppender =
new RumAttributeAppender(
config,
new SessionId(new SessionIdTimeoutHandler()),
"version",
visibleScreenTracker,
connectionUtil);

ReadWriteSpan span = mock(ReadWriteSpan.class);
rumAttributeAppender.onStart(Context.current(), span);
verify(span).setAllAttributes(initialAttributes);

config.updateGlobalAttributes(
attributesBuilder -> attributesBuilder.put("cheese", "cheddar"));

span = mock(ReadWriteSpan.class);
rumAttributeAppender.onStart(Context.current(), span);

Attributes updatedAttributes =
Attributes.of(stringKey("cheese"), "cheddar", longKey("size"), 5L);
verify(span).setAllAttributes(updatedAttributes);
}

@Test
public void appendAttributesOnStart() {
Attributes globalAttributes =
Attributes.of(stringKey("cheese"), "Camembert", longKey("size"), 5L);
Config config = mock(Config.class);
when(config.getApplicationName()).thenReturn("appName");
when(config.getGlobalAttributes()).thenReturn(globalAttributes);

SessionId sessionId = mock(SessionId.class);
when(sessionId.getSessionId()).thenReturn("rumSessionId");
Expand All @@ -115,11 +77,16 @@ public void appendAttributesOnStart() {

RumAttributeAppender rumAttributeAppender =
new RumAttributeAppender(
config, sessionId, "rumVersion", visibleScreenTracker, connectionUtil);
APP_NAME,
() -> globalAttributes,
sessionId,
"rumVersion",
visibleScreenTracker,
connectionUtil);

rumAttributeAppender.onStart(Context.current(), span);
verify(span).setAttribute(RumAttributeAppender.RUM_VERSION_KEY, "rumVersion");
verify(span).setAttribute(RumAttributeAppender.APP_NAME_KEY, "appName");
verify(span).setAttribute(RumAttributeAppender.APP_NAME_KEY, APP_NAME);
verify(span).setAttribute(RumAttributeAppender.SESSION_ID_KEY, "rumSessionId");
verify(span).setAttribute(ResourceAttributes.OS_TYPE, "linux");
verify(span).setAttribute(ResourceAttributes.OS_NAME, "Android");
Expand All @@ -137,9 +104,6 @@ public void appendAttributesOnStart() {

@Test
public void appendAttributes_noCurrentScreens() {
Config config = mock(Config.class);
when(config.getApplicationName()).thenReturn("appName");
when(config.getGlobalAttributes()).thenReturn(Attributes.empty());
SessionId sessionId = mock(SessionId.class);
when(sessionId.getSessionId()).thenReturn("rumSessionId");
when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("unknown");
Expand All @@ -148,7 +112,12 @@ public void appendAttributes_noCurrentScreens() {

RumAttributeAppender rumAttributeAppender =
new RumAttributeAppender(
config, sessionId, "rumVersion", visibleScreenTracker, connectionUtil);
APP_NAME,
Attributes::empty,
sessionId,
"rumVersion",
visibleScreenTracker,
connectionUtil);

rumAttributeAppender.onStart(Context.current(), span);
verify(span).setAttribute(SplunkRum.SCREEN_NAME_KEY, "unknown");
Expand Down
Loading

0 comments on commit dd28be3

Please sign in to comment.