feat(stats): Persist the unique id to redis (#947)

* feat(stats): Persist the unique id to redis

Currently the only place that the unique id for a Spinnaker instance
is persisted is in the hal config. This works for users using halyard
as it gets generated the first time it is needed then gets re-used
on subsequent deployments. (It does have the failure mode of copying
the hal config as then the new instance would have the same id as the
old one.)

This doesn't work for users either deploying without Halyard or deploying
via kleat. In this case there is nowhere to store this uniqueid (as
kleat explicitly treats the config only as input).

To solve this problem, we'll store the unique id in redis. On startup,
we'll check to see if there's a value in redis; if there is we'll use
that, if not we'll use the configured value (and persist it).

* fix(stats): Address code review comments

* fix(stats): Remove duplicated code

echo-telemetry/echo-telemetry.gradle

@@ -20,6 +20,7 @@ dependencies {
   api project(':echo-api')
   implementation project(':echo-model')
   implementation project(':echo-core')
+  implementation "com.netflix.spinnaker.kork:kork-jedis"
   implementation 'com.google.protobuf:protobuf-java-util'
   implementation "com.netflix.spinnaker.kork:kork-proto"
   implementation 'com.netflix.spinnaker.kork:kork-web'
@@ -29,6 +30,7 @@ dependencies {
   implementation "org.apache.commons:commons-lang3"
   testImplementation 'io.mockk:mockk'
   testImplementation 'io.strikt:strikt-core'
+  testImplementation "com.netflix.spinnaker.kork:kork-jedis-test"
 }
 
 test {

echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/InstanceIdSupplier.kt

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2020 Google, LLC
+ *
+ * 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
+ *
+ *   http://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 com.netflix.spinnaker.echo.telemetry
+
+import com.netflix.spinnaker.echo.config.TelemetryConfig
+import com.netflix.spinnaker.kork.jedis.RedisClientSelector
+import com.netflix.spinnaker.kork.jedis.exception.RedisClientNotFound
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
+import org.springframework.stereotype.Component
+import redis.clients.jedis.commands.JedisCommands
+
+val UNIQUE_ID_KEY = "spinnaker:stats:instanceId"
+
+@Component
+@ConditionalOnProperty("stats.enabled")
+class InstanceIdSupplier(
+  private val config: TelemetryConfig.TelemetryConfigProps,
+  redisSelector: RedisClientSelector?
+) {
+  private val log: Logger = LoggerFactory.getLogger(this.javaClass)
+  private val redis = try { redisSelector?.primary("default") } catch (e: RedisClientNotFound) { null }
+  val uniqueId by lazy { getOrSetId(config.instanceId) }
+
+  /**
+   * Returns the current instance id for this Spinnaker instance. If no id is currently set,
+   * sets it to [newId] and returns [newId].
+   */
+  private fun getOrSetId(newId: String): String {
+    return try {
+      redis?.withCommandsClient<String> { c: JedisCommands ->
+        c.setnx(UNIQUE_ID_KEY, newId)
+        c.get(UNIQUE_ID_KEY)
+      }
+    } catch (e: Exception) {
+      log.warn("Error synchronizing unique instance id with redis", e)
+      null
+    } ?: newId
+  }
+}

echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/SpinnakerInstanceDataProvider.kt

@@ -28,14 +28,14 @@ import org.springframework.stereotype.Component
 
 @Component
 @ConditionalOnProperty("stats.enabled")
-class SpinnakerInstanceDataProvider(private val config: TelemetryConfigProps) : TelemetryEventDataProvider {
+class SpinnakerInstanceDataProvider(private val config: TelemetryConfigProps, private val instanceIdSupplier: InstanceIdSupplier) : TelemetryEventDataProvider {
   override fun populateData(echoEvent: EchoEvent, statsEvent: StatsEvent): StatsEvent {
 
     // TelemetryEventListener should ensure this is set
     val applicationId: String = echoEvent.details?.application
       ?: throw IllegalStateException("application not set")
 
-    val instanceId: String = config.instanceId
+    val instanceId: String = instanceIdSupplier.uniqueId
     val hashedInstanceId: String = hash(instanceId)
 
     // We want to ensure it's really hard to guess the application name. Using the instance ID (a

echo-telemetry/src/test/kotlin/com/netflix/spinnaker/echo/telemetry/InstanceIdSupplierTest.kt

@@ -0,0 +1,96 @@
+/*
+ * Copyright 2020 Google, LLC
+ *
+ * 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
+ *
+ *   http://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 com.netflix.spinnaker.echo.telemetry
+
+import com.netflix.spinnaker.echo.config.TelemetryConfig.TelemetryConfigProps
+import com.netflix.spinnaker.kork.jedis.EmbeddedRedis
+import com.netflix.spinnaker.kork.jedis.JedisClientDelegate
+import com.netflix.spinnaker.kork.jedis.RedisClientSelector
+import io.mockk.every
+import io.mockk.mockk
+import org.junit.jupiter.api.Test
+import redis.clients.jedis.Jedis
+import redis.clients.jedis.exceptions.JedisException
+import redis.clients.jedis.util.Pool
+import strikt.api.expectThat
+import strikt.assertions.isEqualTo
+
+class InstanceIdSupplierTest {
+  @Test
+  fun `returns the configured value with no redis backend`() {
+    val instanceIdSupplier = InstanceIdSupplier(withInstanceId("my-id"), null)
+
+    expectThat(instanceIdSupplier.uniqueId).isEqualTo("my-id")
+  }
+
+  @Test
+  fun `returns the configured value when no current value in redis`() {
+    val instanceIdSupplier = InstanceIdSupplier(withInstanceId("my-id"), getRedis())
+
+    expectThat(instanceIdSupplier.uniqueId).isEqualTo("my-id")
+  }
+
+  @Test
+  fun `returns the value in redis if it exists`() {
+    val redis = getRedis()
+    val instanceIdSupplier = InstanceIdSupplier(withInstanceId("my-id"), redis)
+    expectThat(instanceIdSupplier.uniqueId).isEqualTo("my-id")
+
+    val otherInstanceIdSupplier = InstanceIdSupplier(withInstanceId("my-new-id"), redis)
+    expectThat(otherInstanceIdSupplier.uniqueId).isEqualTo("my-id")
+  }
+
+  @Test
+  fun `returns the configured value if the selector is unable to find the configured redis`() {
+    val instanceIdSupplier = InstanceIdSupplier(withInstanceId("my-id"), getMissingRedis())
+    expectThat(instanceIdSupplier.uniqueId).isEqualTo("my-id")
+  }
+
+  @Test
+  fun `returns the configured value if a redis exception occurs`() {
+    val instanceIdSupplier = InstanceIdSupplier(withInstanceId("my-id"), getBadRedis())
+    expectThat(instanceIdSupplier.uniqueId).isEqualTo("my-id")
+  }
+
+  private fun withInstanceId(id: String): TelemetryConfigProps {
+    val result = TelemetryConfigProps()
+    result.instanceId = id
+    return result
+  }
+
+  private fun getRedis(): RedisClientSelector {
+    val embeddedRedis = EmbeddedRedis.embed()
+    val redisClientDelegate = JedisClientDelegate("primaryDefault", embeddedRedis.pool)
+    return RedisClientSelector(listOf(redisClientDelegate))
+  }
+
+  private fun getMissingRedis(): RedisClientSelector {
+    return RedisClientSelector(listOf())
+  }
+
+  private fun getBadRedis(): RedisClientSelector {
+    val jedis = mockk<Jedis>()
+    every { jedis.setnx(any<String>(), any<String>()) } throws JedisException("An error occurred")
+
+    val pool = mockk<Pool<Jedis>>()
+    every { pool.resource } returns jedis
+
+    val delegate = JedisClientDelegate("primaryDefault", pool)
+
+    return RedisClientSelector(listOf(delegate))
+  }
+}

echo-telemetry/src/test/kotlin/com/netflix/spinnaker/echo/telemetry/SpinnakerInstanceDataProviderTest.kt

@@ -42,7 +42,7 @@ class SpinnakerInstanceDataProviderTest {
   @BeforeEach
   fun setUp() {
     config = TelemetryConfigProps()
-    dataProvider = SpinnakerInstanceDataProvider(config)
+    dataProvider = SpinnakerInstanceDataProvider(config, InstanceIdSupplier(config, null))
   }
 
   @Test