feat(rum-core): use etag for fetching config (#439)

* feat(rum-core): use etag for fetching config

store remote config in localStorage

* use sessionStorage

add more tests

* ignore falsy values in setLocalConfig

dev-utils/test-servers.js

@@ -80,8 +80,16 @@ function startApmServerProxy(port = 8001) {
       userResHeaderDecorator(headers) {
         if (!headers['Access-Control-Allow-Origin']) {
           headers['Access-Control-Allow-Origin'] = '*'
+          headers['Access-Control-Expose-Headers'] = 'ETag'
         }
         return headers
+      },
+      proxyReqOptDecorator(proxyReqOpts, srcReq) {
+        const { ifnonematch } = srcReq.query
+        if (ifnonematch) {
+          proxyReqOpts.headers['If-None-Match'] = `"${ifnonematch}"`
+        }
+        return proxyReqOpts
       }
     })
   )

packages/rum-core/src/common/apm-server.js

@@ -80,11 +80,17 @@ class ApmServer {
       headers: {
         'Content-Type': 'application/x-ndjson'
       }
-    })
+    }).then(({ responseText }) => responseText)
   }
 
   _constructError(reason) {
     const { url, status, responseText } = reason
+    /**
+     * The `reason` could be a different type, e.g. an Error
+     */
+    if (typeof status == 'undefined') {
+      return reason
+    }
     let message = url + ' HTTP status: ' + status
     if (__DEV__ && responseText) {
       try {
@@ -127,7 +133,7 @@ class ApmServer {
           if (status === 0 || (status > 399 && status < 600)) {
             reject({ url, status, responseText })
           } else {
-            resolve(responseText)
+            resolve(xhr)
           }
         }
       }
@@ -158,9 +164,26 @@ class ApmServer {
     if (environment) {
       configEndpoint += `&service.environment=${environment}`
     }
+
+    let localConfig = this._configService.getLocalConfig()
+    if (localConfig) {
+      configEndpoint += `&ifnonematch=${localConfig.etag}`
+    }
+
     return this._makeHttpRequest('GET', configEndpoint, { timeout: 5000 })
-      .then(response => {
-        return JSON.parse(response)
+      .then(xhr => {
+        const { status, responseText } = xhr
+        if (status === 304) {
+          return localConfig
+        } else {
+          let remoteConfig = JSON.parse(responseText)
+          const etag = xhr.getResponseHeader('etag')
+          if (etag) {
+            remoteConfig.etag = etag.replace(/["]/g, '')
+            this._configService.setLocalConfig(remoteConfig)
+          }
+          return remoteConfig
+        }
       })
       .catch(reason => {
         const error = this._constructError(reason)

packages/rum-core/src/common/config-service.js

@@ -31,7 +31,7 @@ import {
   getDtHeaderValue
 } from './utils'
 import EventHandler from './event-handler'
-import { CONFIG_CHANGE } from './constants'
+import { CONFIG_CHANGE, LOCAL_CONFIG_KEY } from './constants'
 
 function getConfigFromScript() {
   var script = getCurrentScript()
@@ -260,6 +260,19 @@ class Config {
 
     return errors
   }
+
+  getLocalConfig() {
+    let config = sessionStorage.getItem(LOCAL_CONFIG_KEY)
+    if (config) {
+      return JSON.parse(config)
+    }
+  }
+
+  setLocalConfig(config) {
+    if (config) {
+      sessionStorage.setItem(LOCAL_CONFIG_KEY, JSON.stringify(config))
+    }
+  }
 }
 
 export default Config

packages/rum-core/src/common/constants.js

@@ -103,6 +103,12 @@ const ERROR = 'error'
 const BEFORE_EVENT = ':before'
 const AFTER_EVENT = ':after'
 
+/**
+ * Local Config Key used storing the remote config in the localStorage
+ */
+
+const LOCAL_CONFIG_KEY = 'elastic_apm_config'
+
 export {
   SCHEDULE,
   INVOKE,
@@ -126,5 +132,6 @@ export {
   HISTORY,
   ERROR,
   BEFORE_EVENT,
-  AFTER_EVENT
+  AFTER_EVENT,
+  LOCAL_CONFIG_KEY
 }

packages/rum-core/test/common/apm-server.spec.js

@@ -33,6 +33,7 @@ import { captureBreakdown } from '../../src/performance-monitoring/breakdown'
 import { createServiceFactory } from '../'
 
 const { agentConfig, testConfig } = getGlobalConfig('rum-core')
+import { LOCAL_CONFIG_KEY } from '../../src/common/constants'
 
 function generateTransaction(count, breakdown = false) {
   var result = []
@@ -426,20 +427,49 @@ describe('ApmServer', function() {
 
   if (isVersionInRange(testConfig.stackVersion, '7.3.0')) {
     it('should fetch remote config', async () => {
+      spyOn(configService, 'setLocalConfig')
+      spyOn(configService, 'getLocalConfig')
+
       var config = await apmServer.fetchConfig('nonexistent-service')
-      expect(config).toEqual({})
+      expect(configService.getLocalConfig).toHaveBeenCalled()
+      expect(configService.setLocalConfig).toHaveBeenCalled()
+
+      expect(config).toEqual({ etag: jasmine.any(String) })
 
       config = await apmServer.fetchConfig(
         'nonexistent-service',
         'nonexistent-env'
       )
-      expect(config).toEqual({})
+      expect(config).toEqual({ etag: jasmine.any(String) })
 
       try {
         config = await apmServer.fetchConfig()
       } catch (e) {
         expect(e).toBe('serviceName is required for fetching central config.')
       }
     })
+
+    it('should use local config if available', async () => {
+      configService.setLocalConfig({
+        transaction_sample_rate: '0.5',
+        etag: 'test'
+      })
+
+      apmServer._makeHttpRequest = () => {
+        return Promise.resolve({
+          status: 304
+        })
+      }
+
+      let config = await apmServer.fetchConfig(
+        'nonexistent-service',
+        'nonexistent-env'
+      )
+      expect(config).toEqual({
+        transaction_sample_rate: '0.5',
+        etag: 'test'
+      })
+      sessionStorage.removeItem(LOCAL_CONFIG_KEY)
+    })
   }
 })

packages/rum-core/test/common/config-service.spec.js

@@ -24,6 +24,7 @@
  */
 
 import ConfigService from '../../src/common/config-service'
+import { LOCAL_CONFIG_KEY } from '../../src/common/constants'
 
 describe('ConfigService', function() {
   var configService
@@ -253,4 +254,14 @@ describe('ConfigService', function() {
     )
     expect(configService.get('serverUrl')).toEqual('http://localhost:8080')
   })
+
+  it('should store configuration in sessionConfig', () => {
+    let config = configService.getLocalConfig()
+    expect(config).toBe(undefined)
+    configService.setLocalConfig({ key: 'value' })
+
+    config = configService.getLocalConfig()
+    expect(config).toEqual({ key: 'value' })
+    sessionStorage.removeItem(LOCAL_CONFIG_KEY)
+  })
 })

packages/rum/test/specs/apm-base.spec.js

@@ -313,6 +313,9 @@ describe('ApmBase', function() {
     const apmServer = serviceFactory.getService('ApmServer')
     const configService = serviceFactory.getService('ConfigService')
 
+    spyOn(configService, 'setLocalConfig')
+    spyOn(configService, 'getLocalConfig')
+
     const apmBase = new ApmBase(serviceFactory, !enabled)
     apmBase.init({
       serviceName: 'test-service',
@@ -323,10 +326,19 @@ describe('ApmBase', function() {
 
     function createPayloadCallback(rate) {
       return () => {
-        return Promise.resolve(`{
+        const responseText = `{
           "transaction_sample_rate": "${rate}"
         }
-        `)
+        `
+        return Promise.resolve({
+          responseText,
+          getResponseHeader(headerName) {
+            if (headerName == 'etag') {
+              return '"test"'
+            }
+          },
+          status: 200
+        })
       }
     }