From dd28be3d93ddb9feadde5f66ba92d8d6b76f3c43 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 30 Aug 2022 17:33:06 +0200 Subject: [PATCH] Remove stateful mutable field from Config (#341) * Remove stateful mutable field from Config * non-empty attributes --- .../src/main/java/com/splunk/rum/Config.java | 22 +------ .../com/splunk/rum/RumAttributeAppender.java | 15 +++-- .../java/com/splunk/rum/RumInitializer.java | 13 +++- .../main/java/com/splunk/rum/SplunkRum.java | 22 +++++-- .../test/java/com/splunk/rum/ConfigTest.java | 16 ----- .../splunk/rum/RumAttributeAppenderTest.java | 65 +++++-------------- .../java/com/splunk/rum/SplunkRumTest.java | 34 +++++----- 7 files changed, 76 insertions(+), 111 deletions(-) diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/Config.java b/splunk-otel-android/src/main/java/com/splunk/rum/Config.java index 9fca2d33c..2f0747a54 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/Config.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/Config.java @@ -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; @@ -43,7 +41,7 @@ public class Config { private final boolean crashReportingEnabled; private final boolean networkMonitorEnabled; private final boolean anrDetectionEnabled; - private final AtomicReference globalAttributes = new AtomicReference<>(); + private final Attributes globalAttributes; private final Function spanFilterExporterDecorator; private final boolean slowRenderingDetectionEnabled; private final Duration slowRenderingDetectionPollInterval; @@ -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; @@ -118,7 +116,7 @@ public boolean isSlowRenderingDetectionEnabled() { * instrumentation. */ public Attributes getGlobalAttributes() { - return globalAttributes.get(); + return globalAttributes; } /** Is the network monitoring feature enabled or not. */ @@ -172,20 +170,6 @@ public static Builder builder() { return new Builder(); } - void updateGlobalAttributes(Consumer 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); } diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/RumAttributeAppender.java b/splunk-otel-android/src/main/java/com/splunk/rum/RumAttributeAppender.java index 3916f2dcc..207be3dd3 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/RumAttributeAppender.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/RumAttributeAppender.java @@ -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 APP_NAME_KEY = stringKey("app"); @@ -39,19 +41,22 @@ class RumAttributeAppender implements SpanProcessor { static final AttributeKey SPLUNK_OPERATION_KEY = stringKey("_splunk_operation"); - private final Config config; + private final String applicationName; + private final Supplier globalAttributesSupplier; private final SessionId sessionId; private final String rumVersion; private final VisibleScreenTracker visibleScreenTracker; private final ConnectionUtil connectionUtil; RumAttributeAppender( - Config config, + String applicationName, + Supplier 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; @@ -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); @@ -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); diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/RumInitializer.java b/splunk-otel-android/src/main/java/com/splunk/rum/RumInitializer.java index 9849296b3..657783047 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/RumInitializer.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/RumInitializer.java @@ -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; @@ -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; @@ -62,6 +64,7 @@ class RumInitializer { static final int MAX_ATTRIBUTE_LENGTH = 256 * 128; private final Config config; + private final AtomicReference globalAttributes; private final Application application; private final AppStartupTimer startupTimer; private final List initializationEvents = new ArrayList<>(); @@ -69,6 +72,7 @@ class RumInitializer { 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; @@ -163,7 +167,7 @@ SplunkRum initialize(Supplier 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) { @@ -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())); diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/SplunkRum.java b/splunk-otel-android/src/main/java/com/splunk/rum/SplunkRum.java index a9ea9f979..e71705fb7 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/SplunkRum.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/SplunkRum.java @@ -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; @@ -76,17 +77,20 @@ public class SplunkRum { private final SessionId sessionId; private final OpenTelemetrySdk openTelemetrySdk; - private final Config config; + private final AtomicReference globalAttributes; static { Handler handler = new Handler(Looper.getMainLooper()); startupTimer.detectBackgroundStart(handler); } - SplunkRum(OpenTelemetrySdk openTelemetrySdk, SessionId sessionId, Config config) { + SplunkRum( + OpenTelemetrySdk openTelemetrySdk, + SessionId sessionId, + AtomicReference globalAttributes) { this.openTelemetrySdk = openTelemetrySdk; this.sessionId = sessionId; - this.config = config; + this.globalAttributes = globalAttributes; } /** Create a new {@link Config.Builder} instance. */ @@ -321,7 +325,17 @@ public SplunkRum setGlobalAttribute(AttributeKey key, T value) { * operating on a {@link AttributesBuilder} from the current set. */ public void updateGlobalAttributes(Consumer 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 diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/ConfigTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/ConfigTest.java index d57a8d6e2..aa26296e6 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/ConfigTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/ConfigTest.java @@ -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 = diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/RumAttributeAppenderTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/RumAttributeAppenderTest.java index b6bc63ba1..8f5fb119a 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/RumAttributeAppenderTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/RumAttributeAppenderTest.java @@ -37,6 +37,8 @@ public class RumAttributeAppenderTest { + private static final String APP_NAME = "appName"; + private VisibleScreenTracker visibleScreenTracker; private final ConnectionUtil connectionUtil = mock(ConnectionUtil.class); @@ -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, @@ -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"); @@ -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"); @@ -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"); @@ -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"); diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/SplunkRumTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/SplunkRumTest.java index 2b8729591..838dfbc07 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/SplunkRumTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/SplunkRumTest.java @@ -51,6 +51,7 @@ import java.io.File; import java.time.Duration; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -59,13 +60,10 @@ public class SplunkRumTest { @Rule public OpenTelemetryRule otelTesting = OpenTelemetryRule.create(); + private Tracer tracer; - private final Config config = - Config.builder() - .realm("us0") - .rumAccessToken("token") - .applicationName("appName") - .build(); + private final AtomicReference globalAttributes = + new AtomicReference<>(Attributes.of(stringKey("key"), "value")); @Before public void setup() { @@ -147,7 +145,7 @@ public void addEvent() { new SplunkRum( (OpenTelemetrySdk) otelTesting.getOpenTelemetry(), new SessionId(new SessionIdTimeoutHandler()), - config); + globalAttributes); Attributes attributes = Attributes.of(stringKey("one"), "1", longKey("two"), 2L); splunkRum.addRumEvent("foo", attributes); @@ -164,15 +162,15 @@ public void updateGlobalAttributes() { new SplunkRum( (OpenTelemetrySdk) otelTesting.getOpenTelemetry(), new SessionId(new SessionIdTimeoutHandler()), - config); + globalAttributes); splunkRum.updateGlobalAttributes( - attributesBuilder -> attributesBuilder.put("key", "value")); + attributesBuilder -> attributesBuilder.put("key", "value2")); splunkRum.setGlobalAttribute(longKey("otherKey"), 1234L); assertEquals( - Attributes.of(stringKey("key"), "value", longKey("otherKey"), 1234L), - config.getGlobalAttributes()); + Attributes.of(stringKey("key"), "value2", longKey("otherKey"), 1234L), + globalAttributes.get()); } @Test @@ -187,7 +185,7 @@ public void recordAnr() { new SplunkRum( (OpenTelemetrySdk) otelTesting.getOpenTelemetry(), new SessionId(new SessionIdTimeoutHandler()), - config); + globalAttributes); Attributes expectedAttributes = Attributes.of( @@ -212,7 +210,8 @@ public void addException() { OpenTelemetrySdk testSdk = buildTestSdk(testExporter); SplunkRum splunkRum = - new SplunkRum(testSdk, new SessionId(new SessionIdTimeoutHandler()), config); + new SplunkRum( + testSdk, new SessionId(new SessionIdTimeoutHandler()), globalAttributes); NullPointerException exception = new NullPointerException("oopsie"); Attributes attributes = Attributes.of(stringKey("one"), "1", longKey("two"), 2L); @@ -245,7 +244,7 @@ public void createAndEnd() { new SplunkRum( (OpenTelemetrySdk) otelTesting.getOpenTelemetry(), new SessionId(new SessionIdTimeoutHandler()), - config); + globalAttributes); Span span = splunkRum.startWorkflow("workflow"); Span inner = tracer.spanBuilder("foo").startSpan(); @@ -289,11 +288,12 @@ public void integrateWithBrowserRum() { @Test public void updateLocation() { + AtomicReference globalAttributes = new AtomicReference<>(Attributes.empty()); SplunkRum splunkRum = new SplunkRum( (OpenTelemetrySdk) otelTesting.getOpenTelemetry(), new SessionId(new SessionIdTimeoutHandler()), - config); + globalAttributes); Location location = mock(Location.class); when(location.getLatitude()).thenReturn(42d); @@ -306,10 +306,10 @@ public void updateLocation() { 42d, SplunkRum.LOCATION_LONGITUDE_KEY, 43d), - config.getGlobalAttributes()); + globalAttributes.get()); splunkRum.updateLocation(null); - assertTrue(config.getGlobalAttributes().isEmpty()); + assertTrue(globalAttributes.get().isEmpty()); } }