Merge pull request #2722 from SalesforceCommerceCloud/W-18760780-performancejs

jeremy-jung1 · web-flow · commit 0d75e659ddc2 · 2025-07-16T12:05:35.000-07:00
@W-18760780 Integrate opentelemetry into performance.js
diff --git a/packages/pwa-kit-react-sdk/CHANGELOG.md b/packages/pwa-kit-react-sdk/CHANGELOG.md
@@ -2,6 +2,7 @@
 - Fix the performance logging so that it'll capture all SSR queries, even those that result in errors [#2486](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2486)
 - Created an opentelemetry server for SSR tracer intialization [#2617](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2617)
 - Created opentelemetry.js file with utility functions to log OTel spans and metrics [#2705] (https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2705)
+- Update performance.js with usage of opentelemetry spans and add cleanups [#2722](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2722)
 
 ## v3.10.0 (May 22, 2025)
 - Fix the performance logging util to use the correct delimiter for the server-timing header. [#2225](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2295)
diff --git a/packages/pwa-kit-react-sdk/setup-jest.js b/packages/pwa-kit-react-sdk/setup-jest.js
@@ -40,4 +40,20 @@ jest.mock('@salesforce/pwa-kit-runtime/utils/ssr-config', () => {
 // The global performance object is available in production
 // environments for both the server and the client.
 // It's just the jest environment that this is not available
-global.performance = performance
+if (global.performance) {
+    // global.performance exists but is missing methods from perf_hooks in jest
+    if (typeof performance.mark === 'function' && !global.performance.mark) {
+        global.performance.mark = performance.mark.bind(performance)
+    }
+    if (typeof performance.measure === 'function' && !global.performance.measure) {
+        global.performance.measure = performance.measure.bind(performance)
+    }
+    if (typeof performance.clearMarks === 'function' && !global.performance.clearMarks) {
+        global.performance.clearMarks = performance.clearMarks.bind(performance)
+    }
+    if (typeof performance.clearMeasures === 'function' && !global.performance.clearMeasures) {
+        global.performance.clearMeasures = performance.clearMeasures.bind(performance)
+    }
+} else {
+    global.performance = performance
+}
diff --git a/packages/pwa-kit-react-sdk/src/ssr/server/opentelemetry-server.js b/packages/pwa-kit-react-sdk/src/ssr/server/opentelemetry-server.js
@@ -11,7 +11,7 @@ import {B3Propagator} from '@opentelemetry/propagator-b3'
 import {Resource} from '@opentelemetry/resources'
 import {propagation} from '@opentelemetry/api'
 import logger from '../../utils/logger-instance'
-import {getServiceName, OTEL_CONFIG} from '../../utils/opentelemetry'
+import {getServiceName, getOTELConfig} from '../../utils/opentelemetry-config'
 
 let provider = null
 
@@ -27,7 +27,7 @@ export const initializeServerTracing = (options = {}) => {
     const {
         serviceName = options.serviceName || getServiceName(),
         serviceVersion,
-        enabled = OTEL_CONFIG.enabled
+        enabled = getOTELConfig().enabled
     } = options
 
     // If tracing is disabled, return null without initializing
diff --git a/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js b/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js
@@ -8,6 +8,7 @@
 /**
  * @module progressive-web-sdk/ssr/server/react-rendering
  */
+import {initializeServerTracing, isServerTracingInitialized} from './opentelemetry-server'
 
 import path from 'path'
 import React from 'react'
@@ -122,6 +123,11 @@ export const getLocationSearch = (req, opts = {}) => {
 export const render = async (req, res, next) => {
     const includeServerTimingHeader = '__server_timing' in req.query
     const shouldTrackPerformance = includeServerTimingHeader || process.env.SERVER_TIMING
+
+    if (!isServerTracingInitialized() && shouldTrackPerformance) {
+        initializeServerTracing()
+    }
+
     res.__performanceTimer = new PerformanceTimer({enabled: shouldTrackPerformance})
     res.__performanceTimer.mark(PERFORMANCE_MARKS.total, 'start')
     const AppConfig = getAppConfig()
@@ -222,6 +228,11 @@ export const render = async (req, res, next) => {
         // Here, we use Express's convention to invoke error middleware.
         // Note, we don't have an error handling middleware yet! This is calling the
         // default error handling middleware provided by Express
+
+        if (res.__performanceTimer) {
+            res.__performanceTimer.cleanup()
+        }
+
         return next(e)
     }
 
@@ -244,6 +255,8 @@ export const render = async (req, res, next) => {
         res.set('Cache-Control', NO_CACHE)
     }
 
+    res.__performanceTimer.cleanup()
+
     if (redirectUrl) {
         res.redirect(routerContext.status || 302, redirectUrl)
     } else {
diff --git a/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.test.js b/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.test.js
@@ -739,6 +739,50 @@ describe('The Node SSR Environment', () => {
                     'max-age=0, nocache, nostore, must-revalidate'
                 )
             }
+        },
+        {
+            description: `Performance timer cleanup is called during rendering`,
+            req: {url: '/pwa/', query: {__server_timing: '1'}},
+            mocks: () => {
+                // Mock PerformanceTimer to spy on cleanup
+                const PerformanceTimer = jest.requireActual('../../utils/performance').default
+                const originalCleanup = PerformanceTimer.prototype.cleanup
+                const cleanupSpy = jest.fn(originalCleanup)
+                PerformanceTimer.prototype.cleanup = cleanupSpy
+
+                // Store the spy for assertions
+                global.performanceCleanupSpy = cleanupSpy
+            },
+            assertions: (res) => {
+                expect(res.statusCode).toBe(200)
+                expect(global.performanceCleanupSpy).toHaveBeenCalled()
+                expect(global.performanceCleanupSpy).toHaveBeenCalledTimes(1)
+
+                // Clean up global
+                delete global.performanceCleanupSpy
+            }
+        },
+        {
+            description: `Performance timer cleanup is called even when rendering throws an error`,
+            req: {url: '/unknown-error/'}, // This URL causes an error
+            mocks: () => {
+                // Mock PerformanceTimer to spy on cleanup
+                const PerformanceTimer = jest.requireActual('../../utils/performance').default
+                const originalCleanup = PerformanceTimer.prototype.cleanup
+                const cleanupSpy = jest.fn(originalCleanup)
+                PerformanceTimer.prototype.cleanup = cleanupSpy
+
+                // Store the spy for assertions
+                global.performanceCleanupSpyError = cleanupSpy
+            },
+            assertions: (res) => {
+                expect(res.statusCode).toBe(500)
+                expect(global.performanceCleanupSpyError).toHaveBeenCalled()
+                expect(global.performanceCleanupSpyError).toHaveBeenCalledTimes(1)
+
+                // Clean up global
+                delete global.performanceCleanupSpyError
+            }
         }
     ]
 
diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.js
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2025, Salesforce, Inc.
+ * All rights reserved.
+ * SPDX-License-Identifier: BSD-3-Clause
+ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+const DEFAULT_SERVICE_NAME = 'pwa-kit-react-sdk'
+
+// Only call this function in the server context
+// This wrapper function is necessary because if the config is in the top-level code
+// process will be undefined as it gets executed in the browser context and will throw an uncaught error.
+export const getOTELConfig = () => {
+    return {
+        serviceName: process.env.OTEL_SERVICE_NAME || DEFAULT_SERVICE_NAME,
+        enabled: process.env.OTEL_SDK_ENABLED === 'true',
+        b3TracingEnabled: process.env.OTEL_B3_TRACING_ENABLED === 'true'
+    }
+}
+
+export const getServiceName = () => getOTELConfig().serviceName
diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.test.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.test.js
@@ -0,0 +1,146 @@
+/*
+ * Copyright (c) 2025, Salesforce, Inc.
+ * All rights reserved.
+ * SPDX-License-Identifier: BSD-3-Clause
+ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import {getOTELConfig, getServiceName} from './opentelemetry-config'
+
+// Mock the module to reset cache between tests
+const mockModule = () => {
+    jest.resetModules()
+    return require('./opentelemetry-config')
+}
+
+describe('OpenTelemetry Config', () => {
+    const originalEnv = process.env
+
+    beforeEach(() => {
+        // Reset environment variables
+        process.env = {...originalEnv}
+
+        // Clear module cache to reset _cachedConfig
+        jest.resetModules()
+    })
+
+    afterEach(() => {
+        // Restore original environment
+        process.env = originalEnv
+    })
+
+    describe('getOTELConfig', () => {
+        describe('serviceName', () => {
+            test('should return service name when OTEL_SERVICE_NAME is set', () => {
+                process.env.OTEL_SERVICE_NAME = 'custom-service'
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.serviceName).toBe('custom-service')
+            })
+
+            test('should return default service name when OTEL_SERVICE_NAME is not set', () => {
+                delete process.env.OTEL_SERVICE_NAME
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.serviceName).toBe('pwa-kit-react-sdk')
+            })
+
+            test('should return default service name when OTEL_SERVICE_NAME is empty string', () => {
+                process.env.OTEL_SERVICE_NAME = ''
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.serviceName).toBe('pwa-kit-react-sdk')
+            })
+        })
+
+        describe('enabled', () => {
+            test('should return enabled when OTEL_SDK_ENABLED is "true"', () => {
+                process.env.OTEL_SDK_ENABLED = 'true'
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.enabled).toBe(true)
+            })
+
+            test('should return disabled when OTEL_SDK_ENABLED is not set', () => {
+                delete process.env.OTEL_SDK_ENABLED
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.enabled).toBe(false)
+            })
+
+            test('should return disabled when OTEL_SDK_ENABLED is "false"', () => {
+                process.env.OTEL_SDK_ENABLED = 'false'
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.enabled).toBe(false)
+            })
+
+            test('should return disabled when OTEL_SDK_ENABLED is any non-"true" value', () => {
+                const nonTrueValues = ['yes', '1', 'True', 'TRUE', 'on', 'enabled', '']
+
+                nonTrueValues.forEach((value) => {
+                    process.env.OTEL_SDK_ENABLED = value
+
+                    const {getOTELConfig} = mockModule()
+                    const config = getOTELConfig()
+
+                    expect(config.enabled).toBe(false)
+                })
+            })
+        })
+
+        describe('b3TracingEnabled', () => {
+            test('should return enabled when OTEL_B3_TRACING_ENABLED is "true"', () => {
+                process.env.OTEL_B3_TRACING_ENABLED = 'true'
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.b3TracingEnabled).toBe(true)
+            })
+
+            test('should return disabled when OTEL_B3_TRACING_ENABLED is not set', () => {
+                delete process.env.OTEL_B3_TRACING_ENABLED
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.b3TracingEnabled).toBe(false)
+            })
+
+            test('should return disabled when OTEL_B3_TRACING_ENABLED is "false"', () => {
+                process.env.OTEL_B3_TRACING_ENABLED = 'false'
+
+                const {getOTELConfig} = mockModule()
+                const config = getOTELConfig()
+
+                expect(config.b3TracingEnabled).toBe(false)
+            })
+
+            test('should return disabled when OTEL_B3_TRACING_ENABLED is any non-"true" value', () => {
+                const nonTrueValues = ['yes', '1', 'True', 'TRUE', 'on', 'enabled', '']
+
+                nonTrueValues.forEach((value) => {
+                    process.env.OTEL_B3_TRACING_ENABLED = value
+
+                    const {getOTELConfig} = mockModule()
+                    const config = getOTELConfig()
+
+                    expect(config.b3TracingEnabled).toBe(false)
+                })
+            })
+        })
+    })
+})
diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js
@@ -8,20 +8,12 @@
 import {trace, context, SpanStatusCode} from '@opentelemetry/api'
 import {hrTimeToMilliseconds, hrTimeToTimeStamp} from '@opentelemetry/core'
 import logger from './logger-instance'
-
-const DEFAULT_SERVICE_NAME = 'pwa-kit-react-sdk'
-
-export const OTEL_CONFIG = {
-    serviceName: process.env.OTEL_SERVICE_NAME || DEFAULT_SERVICE_NAME,
-    enabled: process.env.OTEL_SDK_ENABLED === 'true',
-    b3TracingEnabled: process.env.OTEL_B3_TRACING_ENABLED === 'true'
-}
-
-export const getServiceName = () => OTEL_CONFIG.serviceName
+import {getOTELConfig, getServiceName} from './opentelemetry-config'
 
 const logSpanData = (span, event = 'start', res = null) => {
     const spanContext = span.spanContext()
     const startTime = span.startTime
+
     const endTime = event === 'start' ? startTime : span.endTime
     const duration = event === 'start' ? 0 : hrTimeToMilliseconds(span.duration)
 
@@ -44,7 +36,7 @@ const logSpanData = (span, event = 'start', res = null) => {
         links: [],
         start_time: startTime,
         end_time: endTime,
-        forwardTrace: OTEL_CONFIG.b3TracingEnabled
+        forwardTrace: getOTELConfig().b3TracingEnabled
     }
 
     // Inject B3 headers into response if available
diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.test.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.test.js
@@ -103,13 +103,6 @@ describe('OpenTelemetry Utilities', () => {
         opentelemetryUtils = require('./opentelemetry')
     })
 
-    describe('getServiceName', () => {
-        test('should return the service name from config', () => {
-            const serviceName = opentelemetryUtils.getServiceName()
-            expect(serviceName).toBe('pwa-kit-react-sdk')
-        })
-    })
-
     describe('createSpan', () => {
         test('should create a span successfully', () => {
             const result = opentelemetryUtils.createSpan('test-span', {
diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js
diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js
diff --git a/packages/template-retail-react-app/CHANGELOG.md b/packages/template-retail-react-app/CHANGELOG.md
diff --git a/packages/template-retail-react-app/package.json b/packages/template-retail-react-app/package.json
