From 20eb9b43cadcb2325efbda440c8e22173c0aa1f2 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Tue, 1 Jul 2025 19:33:17 -0700 Subject: [PATCH 01/25] initial file --- packages/pwa-kit-react-sdk/setup-jest.js | 2 +- .../src/utils/opentelemetry.js | 305 ++++++++++++++++++ .../src/utils/performance.js | 112 ++++--- 3 files changed, 380 insertions(+), 39 deletions(-) create mode 100644 packages/pwa-kit-react-sdk/src/utils/opentelemetry.js diff --git a/packages/pwa-kit-react-sdk/setup-jest.js b/packages/pwa-kit-react-sdk/setup-jest.js index 2d94d3e2b1..98410a8d3d 100644 --- a/packages/pwa-kit-react-sdk/setup-jest.js +++ b/packages/pwa-kit-react-sdk/setup-jest.js @@ -40,4 +40,4 @@ 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 +// global.performance = performance diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js new file mode 100644 index 0000000000..ab4433b8a1 --- /dev/null +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -0,0 +1,305 @@ +/* + * Copyright (c) 2024, 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 {trace, context, SpanStatusCode} from '@opentelemetry/api' +import {hrTimeToMilliseconds, hrTimeToTimeStamp} from '@opentelemetry/core' +import logger from './logger-instance' + +const SERVICE_NAME = 'pwa-kit-react-sdk' +const tracer = trace.getTracer(SERVICE_NAME) + +function 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) + + // Create the span data object that matches the expected format + const spanData = { + traceId: spanContext.traceId, + parentId: span.parentSpanId, + name: span.name, + id: spanContext.spanId, + kind: span.kind, + timestamp: hrTimeToTimeStamp(startTime), + duration: duration, + attributes: { + 'service.name': SERVICE_NAME, + ...span.attributes, + event: event // Add event type to distinguish start/end + }, + status: {code: event === 'start' ? SpanStatusCode.UNSET : SpanStatusCode.OK}, + events: [], + links: [], + start_time: startTime, + end_time: endTime, + forwardTrace: process.env.DISABLE_B3_TRACING !== 'true' + } + + // Inject B3 headers into response if available + if (res && process.env.DISABLE_B3_TRACING !== 'true' && event === 'start') { + res.setHeader('x-b3-traceid', spanContext.traceId) + res.setHeader('x-b3-spanid', spanContext.spanId) + res.setHeader('x-b3-sampled', '1') + + if (span.parentSpanId) { + res.setHeader('x-b3-parentspanid', span.parentSpanId) + } + } + + // Only log if this is an end event or if it's a start event for a new span + if (event === 'end' || !span.attributes.hasOwnProperty('event')) { + console.info(JSON.stringify(spanData)) + } +} + +/** + * Creates a new span with the given name and options + * @param {string} name - The name of the span + * @param {Object} options - Span options + * @returns {Span} The created span + */ +export const createSpan = (name, options = {}) => { + try { + // Get the current context and active span + const ctx = context.active() + const currentSpan = trace.getSpan(ctx) + + // Create a new span with the current context + const span = tracer.startSpan( + name, + { + ...options, + attributes: { + ...options.attributes, + 'service.name': SERVICE_NAME + } + }, + ctx + ) + + // Set the new span as active + logSpanData(span, 'start') + return trace.setSpan(ctx, span) + } catch (error) { + console.warn(`Failed to create span "${name}":`, error.message) + return null + } +} + +/** + * Creates a child span with the given name and attributes + * @param {string} name - The name of the span + * @param {Object} attributes - The attributes to add to the span + * @returns {Span} The created span + */ +export const createChildSpan = (name, attributes = {}) => { + try { + const ctx = context.active() + const parentSpan = trace.getSpan(ctx) + + // Don't create duplicate spans + if (parentSpan?.attributes?.performance_mark === name) { + return parentSpan + } + + const {performance_mark, performance_detail, ...otherAttributes} = attributes + + const spanAttributes = { + 'service.name': SERVICE_NAME, + ...otherAttributes + } + + if (performance_mark) { + spanAttributes['performance.mark'] = performance_mark + spanAttributes['performance.type'] = 'start' + spanAttributes['performance.detail'] = + typeof performance_detail === 'string' + ? performance_detail + : JSON.stringify(performance_detail) + } + + const span = tracer.startSpan( + name, + { + attributes: spanAttributes + }, + parentSpan ? ctx : undefined + ) + + logSpanData(span, 'start') + return trace.setSpan(ctx, span) + } catch (error) { + logger.error('Error creating OpenTelemetry span', { + namespace: 'opentelemetry', + error: error.message, + stack: error.stack + }) + return null + } +} + +/** + * Ends a span and logs its data + * @param {Span} span - The span to end + */ +export const endSpan = (span) => { + if (!span) { + return + } + + try { + const ctx = context.active() + const parentSpan = trace.getSpan(ctx) + + span.end() + + // Log completion data + logSpanData(span, 'end') + } catch (error) { + logger.error('Error ending OpenTelemetry span', { + namespace: 'opentelemetry', + error: error.message, + stack: error.stack + }) + } +} + +/** + * Creates a span for performance measurement + * @param {string} name - The name of the performance span + * @param {Function} fn - The function to measure + * @param {Object} res - The response object (optional) + * @returns {Promise} The result of the function + */ +export const tracePerformance = async (name, fn, res = null) => { + // Create the root span + const rootSpan = tracer.startSpan(name, { + attributes: { + 'service.name': SERVICE_NAME + } + }) + + // Create a new context with the root span + const ctx = trace.setSpan(context.active(), rootSpan) + + // Log start event + logSpanData(rootSpan, 'start', res) + + try { + // Run the function within the context of the root span + const result = await context.with(ctx, async () => { + try { + return await fn() + } catch (error) { + rootSpan.setStatus({ + code: SpanStatusCode.ERROR, + message: error.message + }) + throw error + } + }) + + rootSpan.end() + + // Log completion data + logSpanData(rootSpan, 'end', res) + + return result + } catch (error) { + rootSpan.end() + + // Log error completion + logSpanData(rootSpan, 'end', res) + + throw error + } +} + +/** + * Traces a performance metric + * @param {string} name - The name of the metric + * @param {number} duration - The duration of the metric in milliseconds + * @param {Object} attributes - Additional attributes for the metric + */ +export const logPerformanceMetric = (name, duration, attributes = {}) => { + try { + const ctx = context.active() + const parentSpan = trace.getSpan(ctx) + + if (!parentSpan) { + logger.warn('No parent span found in context', {namespace: 'opentelemetry'}) + return + } + + // Extract and normalize performance details + const {performance_mark, performance_detail, ...otherAttributes} = attributes + + // Build metric attributes + const metricAttributes = { + 'service.name': SERVICE_NAME, + 'metric.duration': duration, + ...otherAttributes + } + + if (performance_mark) { + metricAttributes['performance.mark'] = performance_mark + metricAttributes['performance.type'] = 'end' + metricAttributes['performance.detail'] = + typeof performance_detail === 'string' + ? performance_detail + : JSON.stringify(performance_detail) + } + + // Create and immediately end the metric span + const span = tracer.startSpan( + name, + { + attributes: metricAttributes + }, + ctx + ) + + const endTime = hrTimeToTimeStamp(process.hrtime()) + span.end() + + // Log completion data + logSpanData(span, 'end') + } catch (error) { + logger.error('Error logging performance metric', { + namespace: 'opentelemetry', + error: error.message, + stack: error.stack + }) + } +} + +/** + * Traces a performance operation + * @param {string} name - The name of the operation + * @param {Function} fn - The function to trace + * @returns {Promise} The result of the function + */ +export const traceChildPerformance = async (name, fn) => { + const span = createChildSpan(name) + if (!span) { + return fn() + } + + try { + const result = await fn() + endSpan(span) + return result + } catch (error) { + span.setStatus({ + code: SpanStatusCode.ERROR, + message: error.message + }) + endSpan(span) + throw error + } +} \ No newline at end of file diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 75e5709d9e..7492191189 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -5,6 +5,8 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ import logger from './logger-instance' +import {createChildSpan, endSpan, logPerformanceMetric} from './opentelemetry' + export const PERFORMANCE_MARKS = { total: 'ssr.total', renderToString: 'ssr.render-to-string', @@ -37,6 +39,7 @@ export default class PerformanceTimer { end: new Map() } this.metrics = [] + this.spans = new Map() } /** @@ -61,17 +64,18 @@ export default class PerformanceTimer { } /** - * A utility function to format and log the performance metrics. - * - * @function - * @private + * Logs all performance metrics */ log() { + // Log each metric once with the standardized format this.metrics.forEach((metric) => { - logger.info(`${metric.name} - ${metric.duration}ms ${metric.detail || ''}`, { - namespace: 'performance' + logPerformanceMetric(metric.name, metric.duration, { + 'performance.detail': metric.detail || '' }) }) + + // Clear the metrics after logging + this.metrics = [] } /** @@ -81,47 +85,79 @@ export default class PerformanceTimer { * @function * @private */ - mark(name, type, options = {}) { - if (!this.enabled) { + mark(name, type, detail = '') { + if (!name || !type || !this.enabled) { return } - if (!name) { - logger.warn('Performance mark cannot be created because the name is undefined.', { - namespace: 'performance' + try { + // Format detail as a string if it's an object + const formattedDetail = typeof detail === 'object' ? JSON.stringify(detail) : detail + + const mark = { + name: `${name}.${type}`, + entryType: 'mark', + startTime: performance.now(), + detail: formattedDetail + } + + performance.mark(mark.name, { + detail: mark.detail }) - return - } - if (type !== this.MARKER_TYPES.START && type !== this.MARKER_TYPES.END) { - logger.warn( - 'Performance mark cannot be created because the type must be either "start" or "end".', - { - namespace: 'performance' + // Only create spans for 'start' events and store them for later use + if (type === 'start') { + if (!this.spans.has(name)) { + const span = createChildSpan(name, { + performance_mark: name, + performance_type: type, + performance_detail: formattedDetail + }) + if (span) { + this.spans.set(name, span) + } } - ) - return - } + } else if (type === 'end') { + const startMark = `${name}.start` + const endMark = `${name}.end` - const timestamp = performance.now() - const isEnd = type === this.MARKER_TYPES.END - const storage = isEnd ? this.marks.end : this.marks.start - storage.set(name, { - name, - timestamp, - detail: options.detail - }) + try { + const measure = performance.measure(name, startMark, endMark) - if (isEnd) { - const startMark = this.marks.start.get(name) - if (startMark) { - const measurement = { - name, - duration: timestamp - startMark.timestamp, - detail: options.detail + // Add the metric to the metrics array for Server-Timing header + this.metrics.push({ + name, + duration: measure.duration, + detail: formattedDetail + }) + + // End the corresponding span if it exists + const span = this.spans.get(name) + if (span) { + endSpan(span) + this.spans.delete(name) + } + + // Clear the marks + performance.clearMarks(startMark) + performance.clearMarks(endMark) + performance.clearMeasures(name) + } catch (error) { + logger.warn('Failed to measure performance mark', { + name, + error: error.message, + startMark, + endMark + }) } - this.metrics.push(measurement) } + } catch (error) { + logger.error('Error creating performance mark', { + name, + type, + error: error.message, + stack: error.stack + }) } } -} +} \ No newline at end of file From 7b349b6df5a93e212ed74354c4d12f2b725313d2 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Wed, 2 Jul 2025 15:55:47 -0700 Subject: [PATCH 02/25] Uncomment global.performance --- packages/pwa-kit-react-sdk/setup-jest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pwa-kit-react-sdk/setup-jest.js b/packages/pwa-kit-react-sdk/setup-jest.js index 98410a8d3d..2d94d3e2b1 100644 --- a/packages/pwa-kit-react-sdk/setup-jest.js +++ b/packages/pwa-kit-react-sdk/setup-jest.js @@ -40,4 +40,4 @@ 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 +global.performance = performance From 332674682443eac21ff495dd39b5ac54c6771c77 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 16:35:43 -0700 Subject: [PATCH 03/25] Some comments and namespacing --- .../src/utils/performance.js | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 7492191189..d95e21807e 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -82,10 +82,18 @@ export default class PerformanceTimer { * This is a utility function to create performance marks. * The data will be used in console logs and the http response header `server-timing`. * - * @function - * @private + * @param {string} name - Unique identifier for the performance measurement. + * Must be the same for both start and end marks of a pair. E.g. 'ssr.render-to-string' + * + * @param {string} type - Mark type, either 'start' or 'end'. 'start' creates spans and browser marks, + * 'end' completes measurement and cleanup. + * + * @param {Object} [options={}] - Optional configuration object + * @param {string|Object} [options.detail=''] - Additional metadata for the mark + * included in logs and tracing attributes. */ - mark(name, type, detail = '') { + mark(name, type, options = {}) { + const {detail = ''} = options if (!name || !type || !this.enabled) { return } @@ -147,7 +155,8 @@ export default class PerformanceTimer { name, error: error.message, startMark, - endMark + endMark, + namespace: 'PerformanceTimer.mark' }) } } @@ -156,7 +165,8 @@ export default class PerformanceTimer { name, type, error: error.message, - stack: error.stack + stack: error.stack, + namespace: 'PerformanceTimer.mark' }) } } From 051ea731dff5ede15fbfd6240f4dd2d66844db75 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 16:41:31 -0700 Subject: [PATCH 04/25] linting --- packages/pwa-kit-react-sdk/src/utils/performance.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index d95e21807e..5d440defa0 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -89,7 +89,7 @@ export default class PerformanceTimer { * 'end' completes measurement and cleanup. * * @param {Object} [options={}] - Optional configuration object - * @param {string|Object} [options.detail=''] - Additional metadata for the mark + * @param {string|Object} [options.detail=''] - Additional metadata for the mark * included in logs and tracing attributes. */ mark(name, type, options = {}) { @@ -170,4 +170,4 @@ export default class PerformanceTimer { }) } } -} \ No newline at end of file +} From f801b5ba2e6eba7dfee1782c44065072349c45ff Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 17:10:11 -0700 Subject: [PATCH 05/25] Some dev only flags and mods --- package.json | 2 +- packages/pwa-kit-react-sdk/src/utils/opentelemetry.js | 8 +++++++- packages/pwa-kit-react-sdk/src/utils/performance.js | 9 +++++++++ packages/pwa-kit-react-sdk/src/utils/performance.test.js | 1 + 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 2c5a5a9707..937da3abf5 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "test:e2e:a11y": "npx playwright test --project=a11y-mobile --project=a11y-desktop", "test:e2e:update-snapshots": "npx playwright test --update-snapshots --project=a11y-mobile --project=a11y-desktop", "test:e2e:ui": "npx playwright test --ui", - "test": "lerna run --stream --concurrency=1 test", + "test": "lerna run test --scope=@salesforce/pwa-kit-react-sdk", "test:max-file-size": "lerna run --stream test:max-file-size", "check-dep-version": "syncpack list-mismatches --types prod,dev" }, diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js index 4837fb89b1..70657a85dd 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -108,14 +108,19 @@ export const createSpan = (name, options = {}) => { */ export const createChildSpan = (name, attributes = {}) => { try { + console.log('(JEREMY) createChildSpan. FLAG A') const tracer = trace.getTracer(SERVICE_NAME) + console.log('(JEREMY) createChildSpan. FLAG B') const ctx = context.active() + console.log('(JEREMY) createChildSpan. FLAG C') const parentSpan = trace.getSpan(ctx) + console.log('(JEREMY) createChildSpan. FLAG D') // Don't create duplicate spans if (parentSpan?.attributes?.performance_mark === name) { return parentSpan } + console.log('(JEREMY) createChildSpan. FLAG E') const {performance_mark, performance_detail, ...otherAttributes} = attributes @@ -140,8 +145,9 @@ export const createChildSpan = (name, attributes = {}) => { }, parentSpan ? ctx : undefined ) - + console.log('(JEREMY) createChildSpan. FLAG F') logSpanData(span, 'start') + console.log('(JEREMY) createChildSpan. FLAG G') return span } catch (error) { logger.error('Error creating OpenTelemetry span', { diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 5d440defa0..a19f6dd120 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -94,13 +94,18 @@ export default class PerformanceTimer { */ mark(name, type, options = {}) { const {detail = ''} = options + console.log('(JEREMY) name: ', name) + console.log('(JEREMY) type: ', type) + console.log('(JEREMY) this.enabled: ', this.enabled) if (!name || !type || !this.enabled) { return } + console.log('(JEREMY) FLAG A') try { // Format detail as a string if it's an object const formattedDetail = typeof detail === 'object' ? JSON.stringify(detail) : detail + console.log('(JEREMY) FLAG B') const mark = { name: `${name}.${type}`, @@ -112,20 +117,24 @@ export default class PerformanceTimer { performance.mark(mark.name, { detail: mark.detail }) + console.log('(JEREMY) FLAG C') // Only create spans for 'start' events and store them for later use if (type === 'start') { + console.log('(JEREMY) FLAG D') if (!this.spans.has(name)) { const span = createChildSpan(name, { performance_mark: name, performance_type: type, performance_detail: formattedDetail }) + console.log('(JEREMY) span: ', span) if (span) { this.spans.set(name, span) } } } else if (type === 'end') { + console.log('(JEREMY) FLAG E') const startMark = `${name}.start` const endMark = `${name}.end` diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index 6ffdf7d964..f10ee0f650 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -23,6 +23,7 @@ describe('PerformanceTimer', () => { test('can be enabled', () => { const timer = new PerformanceTimer({enabled: true}) timer.mark('test', 'start') + console.log('(JEREMY) timer.marks.start.size: ', timer.marks.start.size) expect(timer.marks.start.size).toBe(1) }) From ce666ecf470bb21cfec40002ac473d2971ead514 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 17:19:51 -0700 Subject: [PATCH 06/25] more info --- packages/pwa-kit-react-sdk/src/utils/opentelemetry.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js index 70657a85dd..f10674a616 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -137,7 +137,7 @@ export const createChildSpan = (name, attributes = {}) => { ? performance_detail : JSON.stringify(performance_detail) } - + console.log('(JEREMY) createChildSpan. spanAttributes: ', spanAttributes) const span = tracer.startSpan( name, { @@ -145,7 +145,7 @@ export const createChildSpan = (name, attributes = {}) => { }, parentSpan ? ctx : undefined ) - console.log('(JEREMY) createChildSpan. FLAG F') + console.log('(JEREMY) createChildSpan. FLAG F. span: ', span) logSpanData(span, 'start') console.log('(JEREMY) createChildSpan. FLAG G') return span From ef27e9c1c9b2514980033a8b8159296f277aeeaa Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 22:17:55 -0700 Subject: [PATCH 07/25] More dev --- packages/pwa-kit-react-sdk/src/utils/opentelemetry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js index f10674a616..43b4093050 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -16,7 +16,7 @@ function logSpanData(span, event = 'start', res = null) { const startTime = span.startTime const endTime = event === 'start' ? startTime : span.endTime const duration = event === 'start' ? 0 : hrTimeToMilliseconds(span.duration) - + console.log('(JEREMY) logSpanData. StartTime: ', startTime) // Create the span data object that matches the expected format const spanData = { traceId: spanContext.traceId, From 02cbb5256a478e4b5b19040efeed350ea2ea96c5 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 22:31:42 -0700 Subject: [PATCH 08/25] Update performance.test.js --- .../pwa-kit-react-sdk/src/utils/performance.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index f10ee0f650..ec5696c94f 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -17,22 +17,22 @@ describe('PerformanceTimer', () => { test('is disabled by default', () => { const timer = new PerformanceTimer() timer.mark('test', 'start') - expect(timer.marks.start.size).toBe(0) + expect(timer.spans.size).toBe(0) }) test('can be enabled', () => { const timer = new PerformanceTimer({enabled: true}) timer.mark('test', 'start') - console.log('(JEREMY) timer.marks.start.size: ', timer.marks.start.size) - expect(timer.marks.start.size).toBe(1) + expect(timer.spans.size).toBe(1) + expect(timer.spans.has('test')).toBe(true) }) test('marks can be added for both types', () => { const timer = new PerformanceTimer({enabled: true}) timer.mark('test', 'start') timer.mark('test', 'end') - expect(timer.marks.start.size).toBe(1) - expect(timer.marks.end.size).toBe(1) + expect(timer.spans.size).toBe(0) + expect(timer.metrics).toHaveLength(1) }) test('measurements are created when a pair of marks is added', () => { From bb36629505a2595673788a6829930132838492b2 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 22:48:02 -0700 Subject: [PATCH 09/25] remove some console logs --- packages/pwa-kit-react-sdk/src/utils/opentelemetry.js | 11 ++--------- packages/pwa-kit-react-sdk/src/utils/performance.js | 9 --------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js index 43b4093050..06718fed1f 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -16,7 +16,6 @@ function logSpanData(span, event = 'start', res = null) { const startTime = span.startTime const endTime = event === 'start' ? startTime : span.endTime const duration = event === 'start' ? 0 : hrTimeToMilliseconds(span.duration) - console.log('(JEREMY) logSpanData. StartTime: ', startTime) // Create the span data object that matches the expected format const spanData = { traceId: spanContext.traceId, @@ -108,19 +107,14 @@ export const createSpan = (name, options = {}) => { */ export const createChildSpan = (name, attributes = {}) => { try { - console.log('(JEREMY) createChildSpan. FLAG A') const tracer = trace.getTracer(SERVICE_NAME) - console.log('(JEREMY) createChildSpan. FLAG B') const ctx = context.active() - console.log('(JEREMY) createChildSpan. FLAG C') const parentSpan = trace.getSpan(ctx) - console.log('(JEREMY) createChildSpan. FLAG D') // Don't create duplicate spans if (parentSpan?.attributes?.performance_mark === name) { return parentSpan } - console.log('(JEREMY) createChildSpan. FLAG E') const {performance_mark, performance_detail, ...otherAttributes} = attributes @@ -137,7 +131,7 @@ export const createChildSpan = (name, attributes = {}) => { ? performance_detail : JSON.stringify(performance_detail) } - console.log('(JEREMY) createChildSpan. spanAttributes: ', spanAttributes) + const span = tracer.startSpan( name, { @@ -145,9 +139,8 @@ export const createChildSpan = (name, attributes = {}) => { }, parentSpan ? ctx : undefined ) - console.log('(JEREMY) createChildSpan. FLAG F. span: ', span) + logSpanData(span, 'start') - console.log('(JEREMY) createChildSpan. FLAG G') return span } catch (error) { logger.error('Error creating OpenTelemetry span', { diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index a19f6dd120..5d440defa0 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -94,18 +94,13 @@ export default class PerformanceTimer { */ mark(name, type, options = {}) { const {detail = ''} = options - console.log('(JEREMY) name: ', name) - console.log('(JEREMY) type: ', type) - console.log('(JEREMY) this.enabled: ', this.enabled) if (!name || !type || !this.enabled) { return } - console.log('(JEREMY) FLAG A') try { // Format detail as a string if it's an object const formattedDetail = typeof detail === 'object' ? JSON.stringify(detail) : detail - console.log('(JEREMY) FLAG B') const mark = { name: `${name}.${type}`, @@ -117,24 +112,20 @@ export default class PerformanceTimer { performance.mark(mark.name, { detail: mark.detail }) - console.log('(JEREMY) FLAG C') // Only create spans for 'start' events and store them for later use if (type === 'start') { - console.log('(JEREMY) FLAG D') if (!this.spans.has(name)) { const span = createChildSpan(name, { performance_mark: name, performance_type: type, performance_detail: formattedDetail }) - console.log('(JEREMY) span: ', span) if (span) { this.spans.set(name, span) } } } else if (type === 'end') { - console.log('(JEREMY) FLAG E') const startMark = `${name}.start` const endMark = `${name}.end` From 1e905cf5c084afdd915375a0b73ea61b245f6dcd Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 23:31:56 -0700 Subject: [PATCH 10/25] Updates --- .../pwa-kit-react-sdk/src/utils/performance.js | 4 ---- .../src/utils/performance.test.js | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 5d440defa0..09b8be410c 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -34,10 +34,6 @@ export default class PerformanceTimer { } constructor(options = {}) { this.enabled = options.enabled || false - this.marks = { - start: new Map(), - end: new Map() - } this.metrics = [] this.spans = new Map() } diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index ec5696c94f..730b8acbda 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -1,6 +1,22 @@ /** * @jest-environment node */ + +// Mock OpenTelemetry functions BEFORE any imports +jest.mock('./opentelemetry', () => ({ + createChildSpan: jest.fn((name, attributes) => ({ + spanContext: () => ({ + traceId: 'test-trace-id', + spanId: 'test-span-id' + }), + name, + attributes, + end: jest.fn() + })), + endSpan: jest.fn(), + logPerformanceMetric: jest.fn() +})) + /* * Copyright (c) 2024, Salesforce, Inc. * All rights reserved. From 0dd809dc1d8af8adcedb4fa28fcd3213d4383fb1 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Mon, 7 Jul 2025 23:38:09 -0700 Subject: [PATCH 11/25] Update performance.test.js --- packages/pwa-kit-react-sdk/src/utils/performance.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index 730b8acbda..068c78d539 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -1,5 +1,8 @@ -/** - * @jest-environment node +/* + * 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 */ // Mock OpenTelemetry functions BEFORE any imports @@ -25,7 +28,6 @@ jest.mock('./opentelemetry', () => ({ */ // The @jest-environment comment block *MUST* be the first line of the file for the tests to pass. // That conflicts with the monorepo header rule, so we must disable the rule! -/* eslint-disable header/header */ import PerformanceTimer from './performance' From 1e586d9ecf0e001eefc9584d60d0638cff9c5b82 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Tue, 8 Jul 2025 00:34:37 -0700 Subject: [PATCH 12/25] Dev mod --- .github/actions/unit_tests/action.yml | 2 +- packages/pwa-kit-react-sdk/src/utils/performance.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/unit_tests/action.yml b/.github/actions/unit_tests/action.yml index 77059c2a0e..be33f7d9ec 100644 --- a/.github/actions/unit_tests/action.yml +++ b/.github/actions/unit_tests/action.yml @@ -20,5 +20,5 @@ runs: # root and *also* on the PWA package. This section is run on both. # Always run fast unit tests - CI is flaky, so let's try three times! - npm run test || npm run test || npm run test + npm run test shell: bash diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index 068c78d539..e8a22f03c8 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -45,7 +45,7 @@ describe('PerformanceTimer', () => { expect(timer.spans.has('test')).toBe(true) }) - test('marks can be added for both types', () => { + test('spans can be added for both types', () => { const timer = new PerformanceTimer({enabled: true}) timer.mark('test', 'start') timer.mark('test', 'end') From 48ff1b9affd38dd66355f1a9aadc2498d4117392 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Wed, 9 Jul 2025 16:01:04 -0700 Subject: [PATCH 13/25] Add tests and more functionality --- .../src/utils/performance.js | 118 +++++++++++--- .../src/utils/performance.test.js | 149 +++++++++++++++++- 2 files changed, 243 insertions(+), 24 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 09b8be410c..1aafc1cb41 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -36,6 +36,8 @@ export default class PerformanceTimer { this.enabled = options.enabled || false this.metrics = [] this.spans = new Map() + this.spanTimeouts = new Map() + this.maxSpanDuration = options.maxSpanDuration || 30000 // 30 seconds default } /** @@ -94,23 +96,21 @@ export default class PerformanceTimer { return } + if (type !== this.MARKER_TYPES.START && type !== this.MARKER_TYPES.END) { + logger.warn('Invalid mark type', {type, name, namespace: 'PerformanceTimer.mark'}) + return + } + try { // Format detail as a string if it's an object const formattedDetail = typeof detail === 'object' ? JSON.stringify(detail) : detail - const mark = { - name: `${name}.${type}`, - entryType: 'mark', - startTime: performance.now(), + global.performance.mark(`${name}.${type}`, { detail: formattedDetail - } - - performance.mark(mark.name, { - detail: mark.detail }) // Only create spans for 'start' events and store them for later use - if (type === 'start') { + if (type === this.MARKER_TYPES.START) { if (!this.spans.has(name)) { const span = createChildSpan(name, { performance_mark: name, @@ -119,14 +119,20 @@ export default class PerformanceTimer { }) if (span) { this.spans.set(name, span) + + // Set up automatic cleanup for orphaned spans + const timeoutId = setTimeout(() => { + this._cleanupOrphanedSpan(name, 'timeout') + }, this.maxSpanDuration) + this.spanTimeouts.set(name, timeoutId) } } - } else if (type === 'end') { - const startMark = `${name}.start` - const endMark = `${name}.end` + } else if (type === this.MARKER_TYPES.END) { + const startMark = `${name}.${this.MARKER_TYPES.START}` + const endMark = `${name}.${this.MARKER_TYPES.END}` try { - const measure = performance.measure(name, startMark, endMark) + const measure = global.performance.measure(name, startMark, endMark) // Add the metric to the metrics array for Server-Timing header this.metrics.push({ @@ -135,17 +141,24 @@ export default class PerformanceTimer { detail: formattedDetail }) - // End the corresponding span if it exists + // End the corresponding span if it exists and clear timeout const span = this.spans.get(name) if (span) { endSpan(span) this.spans.delete(name) + + // Clear the timeout since span completed normally + const timeoutId = this.spanTimeouts.get(name) + if (timeoutId) { + clearTimeout(timeoutId) + this.spanTimeouts.delete(name) + } } // Clear the marks - performance.clearMarks(startMark) - performance.clearMarks(endMark) - performance.clearMeasures(name) + global.performance.clearMarks(startMark) + global.performance.clearMarks(endMark) + global.performance.clearMeasures(name) } catch (error) { logger.warn('Failed to measure performance mark', { name, @@ -157,13 +170,74 @@ export default class PerformanceTimer { } } } catch (error) { - logger.error('Error creating performance mark', { + if (error.name === 'SyntaxError') { + logger.warn('Invalid performance mark name', {name, error: error.message}) + } else { + logger.error('Error creating performance mark', { + name, + type, + error: error.message, + stack: error.stack, + namespace: 'PerformanceTimer.mark' + }) + } + } + } + + /** + * Helper method to clean up a specific orphaned span + * @private + */ + _cleanupOrphanedSpan(name, reason = 'manual') { + const span = this.spans.get(name) + if (span) { + logger.warn('Cleaning up orphaned span', { name, - type, - error: error.message, - stack: error.stack, - namespace: 'PerformanceTimer.mark' + error: 'Deleting orphaned span (reason: ' + reason + ' cleanup)', + namespace: 'PerformanceTimer._cleanupOrphanedSpan' }) + endSpan(span) + this.spans.delete(name) + } + + // Clear the timeout + const timeoutId = this.spanTimeouts.get(name) + if (timeoutId) { + clearTimeout(timeoutId) + this.spanTimeouts.delete(name) + } + } + + /** + * Clean up all orphaned spans and clear all timeouts + * Call this when the timer is no longer needed or when you want to force cleanup + */ + cleanup() { + // Clean up any orphaned spans + this.spans.forEach((span, name) => { + this._cleanupOrphanedSpan(name, 'manual_cleanup') + }) + + // Clear any remaining timeouts + this.spanTimeouts.forEach((timeoutId) => { + clearTimeout(timeoutId) + }) + this.spanTimeouts.clear() + + // Clear metrics as well + this.metrics = [] + } + + /** + * Get information about current spans (useful for debugging) + * @returns {Object} Information about active spans and timeouts + */ + getSpanInfo() { + return { + activeSpans: Array.from(this.spans.keys()), + activeTimeouts: this.spanTimeouts.size, + spanCount: this.spans.size, + metricCount: this.metrics.length } } } diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index e8a22f03c8..067ad741c9 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -45,12 +45,11 @@ describe('PerformanceTimer', () => { expect(timer.spans.has('test')).toBe(true) }) - test('spans can be added for both types', () => { + test('spans are removed when the end mark is added', () => { const timer = new PerformanceTimer({enabled: true}) timer.mark('test', 'start') timer.mark('test', 'end') expect(timer.spans.size).toBe(0) - expect(timer.metrics).toHaveLength(1) }) test('measurements are created when a pair of marks is added', () => { @@ -61,4 +60,150 @@ describe('PerformanceTimer', () => { expect(timer.metrics[0].name).toBe('test') expect(parseFloat(timer.metrics[0].duration)).toBeGreaterThan(0) }) + + test('handles multiple measurements', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test1', 'start') + timer.mark('test1', 'end') + timer.mark('test2', 'start') + timer.mark('test2', 'end') + + expect(timer.metrics).toHaveLength(2) + expect(timer.metrics[0].name).toBe('test1') + expect(timer.metrics[1].name).toBe('test2') + expect(timer.spans.size).toBe(0) + }) + + test('handles string detail correctly', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start', {detail: 'test detail'}) + timer.mark('test', 'end', {detail: 'end detail'}) + + expect(timer.metrics).toHaveLength(1) + expect(timer.metrics[0].detail).toBe('end detail') + }) + + test('handles object detail correctly', () => { + const timer = new PerformanceTimer({enabled: true}) + const detailObj = {key: 'value', nested: {data: 123}} + timer.mark('test', 'start', {detail: detailObj}) + timer.mark('test', 'end', {detail: detailObj}) + + expect(timer.metrics).toHaveLength(1) + expect(timer.metrics[0].detail).toBe(JSON.stringify(detailObj)) + }) + + test('buildServerTimingHeader returns empty string for no metrics', () => { + const timer = new PerformanceTimer({enabled: true}) + expect(timer.buildServerTimingHeader()).toBe('') + }) + + test('buildServerTimingHeader formats metrics correctly', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test1', 'start') + timer.mark('test1', 'end') + timer.mark('test2', 'start') + timer.mark('test2', 'end') + + const header = timer.buildServerTimingHeader() + expect(header).toMatch(/test1;dur=\d+\.\d+/) + expect(header).toMatch(/test2;dur=\d+\.\d+/) + expect(header).toContain(', ') + }) + + test('log method clears metrics after logging', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start') + timer.mark('test', 'end') + + expect(timer.metrics).toHaveLength(1) + timer.log() + expect(timer.metrics).toHaveLength(0) + }) + + test('handles end mark without start mark gracefully', () => { + const timer = new PerformanceTimer({enabled: true}) + + // This should not throw, but should log a warning + expect(() => { + timer.mark('test', 'end') + }).not.toThrow() + + expect(timer.metrics).toHaveLength(0) + expect(timer.spans.size).toBe(0) + }) + + test('handles start mark without end mark', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start') + + expect(timer.spans.size).toBe(1) + expect(timer.metrics).toHaveLength(0) + }) + + test('ignores marks when disabled', () => { + const timer = new PerformanceTimer({enabled: false}) + timer.mark('test', 'start') + timer.mark('test', 'end') + + expect(timer.spans.size).toBe(0) + expect(timer.metrics).toHaveLength(0) + }) + + test('ignores marks with empty name', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('', 'start') + timer.mark(null, 'start') + timer.mark(undefined, 'start') + + expect(timer.spans.size).toBe(0) + expect(timer.metrics).toHaveLength(0) + }) + + test('ignores marks with empty type', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', '') + timer.mark('test', null) + timer.mark('test', undefined) + + expect(timer.spans.size).toBe(0) + expect(timer.metrics).toHaveLength(0) + }) + + test('creates spans only for start marks', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test1', 'start') + timer.mark('test2', 'start') + + expect(timer.spans.size).toBe(2) + expect(timer.spans.has('test1')).toBe(true) + expect(timer.spans.has('test2')).toBe(true) + }) + + test('does not create duplicate spans for same name', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start') + timer.mark('test', 'start') // Duplicate + + expect(timer.spans.size).toBe(1) + expect(timer.spans.has('test')).toBe(true) + }) + + test('works with default options', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start') // No options parameter + timer.mark('test', 'end') + + expect(timer.metrics).toHaveLength(1) + expect(timer.metrics[0].detail).toBe('') + }) + + test('preserves detail from end mark in metrics', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start', {detail: 'start detail'}) + timer.mark('test', 'end', {detail: 'end detail'}) + + expect(timer.metrics).toHaveLength(1) + expect(timer.metrics[0].detail).toBe('end detail') + }) }) From dfb8812ebebaaa52be331d0f698f4e0f94da4d6e Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 14:28:55 -0700 Subject: [PATCH 14/25] Fix tests and add cleanup --- packages/pwa-kit-react-sdk/setup-jest.js | 23 ++++++++++++++++++- .../src/ssr/server/react-rendering.js | 11 +++++++-- .../src/utils/performance.js | 13 ++++------- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/packages/pwa-kit-react-sdk/setup-jest.js b/packages/pwa-kit-react-sdk/setup-jest.js index 2d94d3e2b1..8d40a15a03 100644 --- a/packages/pwa-kit-react-sdk/setup-jest.js +++ b/packages/pwa-kit-react-sdk/setup-jest.js @@ -40,4 +40,25 @@ 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 +} + +// This is needed for spans created in jest environment to not be no-op spans, otherwise the spans will lack attributes +import {BasicTracerProvider} from '@opentelemetry/sdk-trace-base' +const provider = new BasicTracerProvider() +provider.register() \ No newline at end of file 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 index 780b600a3d..0c5f810f24 100644 --- a/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js +++ b/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js @@ -222,6 +222,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) } @@ -233,8 +238,7 @@ export const render = async (req, res, next) => { res.__performanceTimer.mark(PERFORMANCE_MARKS.renderToString, 'end') res.__performanceTimer.mark(PERFORMANCE_MARKS.total, 'end') - res.__performanceTimer.log() - + if (includeServerTimingHeader) { res.setHeader('Server-Timing', res.__performanceTimer.buildServerTimingHeader()) @@ -243,6 +247,9 @@ export const render = async (req, res, next) => { // cache headers set by individual page components res.set('Cache-Control', NO_CACHE) } + + res.__performanceTimer.log() + res.__performanceTimer.cleanup() if (redirectUrl) { res.redirect(routerContext.status || 302, redirectUrl) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 1aafc1cb41..90c40cc7a5 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -71,9 +71,6 @@ export default class PerformanceTimer { 'performance.detail': metric.detail || '' }) }) - - // Clear the metrics after logging - this.metrics = [] } /** @@ -105,7 +102,7 @@ export default class PerformanceTimer { // Format detail as a string if it's an object const formattedDetail = typeof detail === 'object' ? JSON.stringify(detail) : detail - global.performance.mark(`${name}.${type}`, { + performance.mark(`${name}.${type}`, { detail: formattedDetail }) @@ -132,7 +129,7 @@ export default class PerformanceTimer { const endMark = `${name}.${this.MARKER_TYPES.END}` try { - const measure = global.performance.measure(name, startMark, endMark) + const measure = performance.measure(name, startMark, endMark) // Add the metric to the metrics array for Server-Timing header this.metrics.push({ @@ -156,9 +153,9 @@ export default class PerformanceTimer { } // Clear the marks - global.performance.clearMarks(startMark) - global.performance.clearMarks(endMark) - global.performance.clearMeasures(name) + performance.clearMarks(startMark) + performance.clearMarks(endMark) + performance.clearMeasures(name) } catch (error) { logger.warn('Failed to measure performance mark', { name, From 20b8c96e25de36d22a7751b30f20709a9408646f Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 14:29:13 -0700 Subject: [PATCH 15/25] linting --- packages/pwa-kit-react-sdk/setup-jest.js | 2 +- .../pwa-kit-react-sdk/src/ssr/server/react-rendering.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/pwa-kit-react-sdk/setup-jest.js b/packages/pwa-kit-react-sdk/setup-jest.js index 8d40a15a03..e52cc2f789 100644 --- a/packages/pwa-kit-react-sdk/setup-jest.js +++ b/packages/pwa-kit-react-sdk/setup-jest.js @@ -61,4 +61,4 @@ if (global.performance) { // This is needed for spans created in jest environment to not be no-op spans, otherwise the spans will lack attributes import {BasicTracerProvider} from '@opentelemetry/sdk-trace-base' const provider = new BasicTracerProvider() -provider.register() \ No newline at end of file +provider.register() 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 index 0c5f810f24..14b35f2316 100644 --- a/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js +++ b/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js @@ -222,11 +222,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) } @@ -238,7 +238,7 @@ export const render = async (req, res, next) => { res.__performanceTimer.mark(PERFORMANCE_MARKS.renderToString, 'end') res.__performanceTimer.mark(PERFORMANCE_MARKS.total, 'end') - + if (includeServerTimingHeader) { res.setHeader('Server-Timing', res.__performanceTimer.buildServerTimingHeader()) @@ -247,7 +247,7 @@ export const render = async (req, res, next) => { // cache headers set by individual page components res.set('Cache-Control', NO_CACHE) } - + res.__performanceTimer.log() res.__performanceTimer.cleanup() From 1d9a90574d4cad8f9c40a8f4a031f87736d565a1 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 14:46:43 -0700 Subject: [PATCH 16/25] fix tests --- .../src/ssr/server/react-rendering.js | 2 +- .../src/ssr/server/react-rendering.test.js | 44 +++++ .../src/utils/performance.test.js | 186 ++++++++++++++++-- 3 files changed, 211 insertions(+), 21 deletions(-) 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 index 14b35f2316..4d0b79b4c8 100644 --- a/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js +++ b/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js @@ -238,6 +238,7 @@ export const render = async (req, res, next) => { res.__performanceTimer.mark(PERFORMANCE_MARKS.renderToString, 'end') res.__performanceTimer.mark(PERFORMANCE_MARKS.total, 'end') + res.__performanceTimer.log() if (includeServerTimingHeader) { res.setHeader('Server-Timing', res.__performanceTimer.buildServerTimingHeader()) @@ -248,7 +249,6 @@ export const render = async (req, res, next) => { res.set('Cache-Control', NO_CACHE) } - res.__performanceTimer.log() res.__performanceTimer.cleanup() if (redirectUrl) { 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 index 664613c593..d61217e91b 100644 --- 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/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index 067ad741c9..82804b2ecb 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -5,7 +5,6 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ -// Mock OpenTelemetry functions BEFORE any imports jest.mock('./opentelemetry', () => ({ createChildSpan: jest.fn((name, attributes) => ({ spanContext: () => ({ @@ -20,15 +19,6 @@ jest.mock('./opentelemetry', () => ({ logPerformanceMetric: jest.fn() })) -/* - * Copyright (c) 2024, 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 - */ -// The @jest-environment comment block *MUST* be the first line of the file for the tests to pass. -// That conflicts with the monorepo header rule, so we must disable the rule! - import PerformanceTimer from './performance' describe('PerformanceTimer', () => { @@ -111,16 +101,6 @@ describe('PerformanceTimer', () => { expect(header).toContain(', ') }) - test('log method clears metrics after logging', () => { - const timer = new PerformanceTimer({enabled: true}) - timer.mark('test', 'start') - timer.mark('test', 'end') - - expect(timer.metrics).toHaveLength(1) - timer.log() - expect(timer.metrics).toHaveLength(0) - }) - test('handles end mark without start mark gracefully', () => { const timer = new PerformanceTimer({enabled: true}) @@ -206,4 +186,170 @@ describe('PerformanceTimer', () => { expect(timer.metrics).toHaveLength(1) expect(timer.metrics[0].detail).toBe('end detail') }) + + describe('cleanup functionality', () => { + test('cleanup clears all spans', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test1', 'start') + timer.mark('test2', 'start') + timer.mark('test3', 'start') + + expect(timer.spans.size).toBe(3) + + timer.cleanup() + + expect(timer.spans.size).toBe(0) + }) + + test('cleanup clears all timeouts', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test1', 'start') + timer.mark('test2', 'start') + + expect(timer.spanTimeouts.size).toBe(2) + + timer.cleanup() + + expect(timer.spanTimeouts.size).toBe(0) + }) + + test('cleanup clears all metrics', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test1', 'start') + timer.mark('test1', 'end') + timer.mark('test2', 'start') + timer.mark('test2', 'end') + + expect(timer.metrics).toHaveLength(2) + + timer.cleanup() + + expect(timer.metrics).toHaveLength(0) + }) + + test('cleanup works when there are no spans/timeouts/metrics', () => { + const timer = new PerformanceTimer({enabled: true}) + + expect(() => { + timer.cleanup() + }).not.toThrow() + + expect(timer.spans.size).toBe(0) + expect(timer.spanTimeouts.size).toBe(0) + expect(timer.metrics).toHaveLength(0) + }) + + test('cleanup works when timer is disabled', () => { + const timer = new PerformanceTimer({enabled: false}) + timer.mark('test', 'start') // This won't create anything since disabled + + expect(() => { + timer.cleanup() + }).not.toThrow() + + expect(timer.spans.size).toBe(0) + expect(timer.spanTimeouts.size).toBe(0) + expect(timer.metrics).toHaveLength(0) + }) + }) + + describe('timeout and orphaned span handling', () => { + beforeEach(() => { + jest.useFakeTimers() + }) + + afterEach(() => { + jest.useRealTimers() + }) + + test('orphaned spans are cleaned up after maxSpanDuration', () => { + const timer = new PerformanceTimer({enabled: true, maxSpanDuration: 1000}) + const cleanupSpy = jest.spyOn(timer, '_cleanupOrphanedSpan') + + timer.mark('test', 'start') + + expect(timer.spans.size).toBe(1) + expect(timer.spanTimeouts.size).toBe(1) + + // Fast forward time to trigger timeout + jest.advanceTimersByTime(1001) + + expect(cleanupSpy).toHaveBeenCalledWith('test', 'timeout') + expect(timer.spans.size).toBe(0) + expect(timer.spanTimeouts.size).toBe(0) + }) + + test('timeouts are cleared when spans end normally', () => { + const timer = new PerformanceTimer({enabled: true, maxSpanDuration: 1000}) + const cleanupSpy = jest.spyOn(timer, '_cleanupOrphanedSpan') + + timer.mark('test', 'start') + timer.mark('test', 'end') + + expect(timer.spans.size).toBe(0) + expect(timer.spanTimeouts.size).toBe(0) + + // Fast forward time - should not trigger cleanup since span ended normally + jest.advanceTimersByTime(1001) + + expect(cleanupSpy).not.toHaveBeenCalled() + }) + + test('multiple spans can have independent timeouts', () => { + const timer = new PerformanceTimer({enabled: true, maxSpanDuration: 1000}) + const cleanupSpy = jest.spyOn(timer, '_cleanupOrphanedSpan') + + timer.mark('test1', 'start') + timer.mark('test2', 'start') + timer.mark('test3', 'start') + + expect(timer.spans.size).toBe(3) + expect(timer.spanTimeouts.size).toBe(3) + + // End one span normally + timer.mark('test2', 'end') + expect(timer.spans.size).toBe(2) + expect(timer.spanTimeouts.size).toBe(2) + + // Trigger timeouts for remaining spans + jest.advanceTimersByTime(1001) + + expect(cleanupSpy).toHaveBeenCalledTimes(2) + expect(cleanupSpy).toHaveBeenCalledWith('test1', 'timeout') + expect(cleanupSpy).toHaveBeenCalledWith('test3', 'timeout') + expect(timer.spans.size).toBe(0) + expect(timer.spanTimeouts.size).toBe(0) + }) + + test('cleanup clears all timeouts to prevent them from firing', () => { + const timer = new PerformanceTimer({enabled: true, maxSpanDuration: 1000}) + const cleanupSpy = jest.spyOn(timer, '_cleanupOrphanedSpan') + + timer.mark('test1', 'start') + timer.mark('test2', 'start') + + // Cleanup before timeouts fire + timer.cleanup() + + // Fast forward time - timeouts should not fire since they were cleared + jest.advanceTimersByTime(1001) + + // _cleanupOrphanedSpan should only be called from cleanup, not from timeouts + expect(cleanupSpy).toHaveBeenCalledTimes(2) + expect(cleanupSpy).toHaveBeenCalledWith('test1', 'manual_cleanup') + expect(cleanupSpy).toHaveBeenCalledWith('test2', 'manual_cleanup') + }) + }) + + test('log can be called multiple times', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start') + timer.mark('test', 'end') + + expect(() => { + timer.log() + timer.log() + timer.log() + }).not.toThrow() + }) }) From 7171b8f04213e3ccb04aa2ac23c5af8d1f2a2410 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 15:16:01 -0700 Subject: [PATCH 17/25] remove dev only changes and add more tests --- .github/actions/unit_tests/action.yml | 2 +- package.json | 2 +- packages/pwa-kit-react-sdk/CHANGELOG.md | 1 + .../src/utils/performance.js | 13 -- .../src/utils/performance.test.js | 197 ++++++++++++++++-- 5 files changed, 178 insertions(+), 37 deletions(-) diff --git a/.github/actions/unit_tests/action.yml b/.github/actions/unit_tests/action.yml index be33f7d9ec..77059c2a0e 100644 --- a/.github/actions/unit_tests/action.yml +++ b/.github/actions/unit_tests/action.yml @@ -20,5 +20,5 @@ runs: # root and *also* on the PWA package. This section is run on both. # Always run fast unit tests - CI is flaky, so let's try three times! - npm run test + npm run test || npm run test || npm run test shell: bash diff --git a/package.json b/package.json index 937da3abf5..2c5a5a9707 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "test:e2e:a11y": "npx playwright test --project=a11y-mobile --project=a11y-desktop", "test:e2e:update-snapshots": "npx playwright test --update-snapshots --project=a11y-mobile --project=a11y-desktop", "test:e2e:ui": "npx playwright test --ui", - "test": "lerna run test --scope=@salesforce/pwa-kit-react-sdk", + "test": "lerna run --stream --concurrency=1 test", "test:max-file-size": "lerna run --stream test:max-file-size", "check-dep-version": "syncpack list-mismatches --types prod,dev" }, diff --git a/packages/pwa-kit-react-sdk/CHANGELOG.md b/packages/pwa-kit-react-sdk/CHANGELOG.md index 020339a534..c4fd88a56e 100644 --- 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/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 90c40cc7a5..68db711f63 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -224,17 +224,4 @@ export default class PerformanceTimer { // Clear metrics as well this.metrics = [] } - - /** - * Get information about current spans (useful for debugging) - * @returns {Object} Information about active spans and timeouts - */ - getSpanInfo() { - return { - activeSpans: Array.from(this.spans.keys()), - activeTimeouts: this.spanTimeouts.size, - spanCount: this.spans.size, - metricCount: this.metrics.length - } - } } diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index 82804b2ecb..cc597036a5 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -238,19 +238,6 @@ describe('PerformanceTimer', () => { expect(timer.spanTimeouts.size).toBe(0) expect(timer.metrics).toHaveLength(0) }) - - test('cleanup works when timer is disabled', () => { - const timer = new PerformanceTimer({enabled: false}) - timer.mark('test', 'start') // This won't create anything since disabled - - expect(() => { - timer.cleanup() - }).not.toThrow() - - expect(timer.spans.size).toBe(0) - expect(timer.spanTimeouts.size).toBe(0) - expect(timer.metrics).toHaveLength(0) - }) }) describe('timeout and orphaned span handling', () => { @@ -341,15 +328,181 @@ describe('PerformanceTimer', () => { }) }) - test('log can be called multiple times', () => { - const timer = new PerformanceTimer({enabled: true}) - timer.mark('test', 'start') - timer.mark('test', 'end') + describe('error handling and edge cases', () => { + test('handles performance.measure errors gracefully', () => { + const timer = new PerformanceTimer({enabled: true}) - expect(() => { - timer.log() - timer.log() - timer.log() - }).not.toThrow() + // Mock performance.measure to throw an error + const originalMeasure = performance.measure + performance.measure = jest.fn().mockImplementation(() => { + throw new Error('Measure failed') + }) + + timer.mark('test', 'start') + + expect(() => { + timer.mark('test', 'end') + }).not.toThrow() + + expect(timer.metrics).toHaveLength(0) + // The span is still there because cleanup only happens on successful measure + expect(timer.spans.size).toBe(1) + + // Restore original method + performance.measure = originalMeasure + }) + + test('handles SyntaxError in performance.mark gracefully', () => { + const timer = new PerformanceTimer({enabled: true}) + + // Mock performance.mark to throw a SyntaxError + const originalMark = performance.mark + performance.mark = jest.fn().mockImplementation(() => { + const error = new Error('Invalid mark name') + error.name = 'SyntaxError' + throw error + }) + + expect(() => { + timer.mark('test', 'start') + }).not.toThrow() + + expect(timer.spans.size).toBe(0) + + // Restore original method + performance.mark = originalMark + }) + + test('handles generic errors in performance.mark gracefully', () => { + const timer = new PerformanceTimer({enabled: true}) + + // Mock performance.mark to throw a generic error + const originalMark = performance.mark + performance.mark = jest.fn().mockImplementation(() => { + throw new Error('Generic error') + }) + + expect(() => { + timer.mark('test', 'start') + }).not.toThrow() + + expect(timer.spans.size).toBe(0) + + // Restore original method + performance.mark = originalMark + }) + + test('handles invalid mark types', () => { + const timer = new PerformanceTimer({enabled: true}) + + timer.mark('test', 'invalid-type') + timer.mark('test', 'middle') + timer.mark('test', 'pause') + + expect(timer.spans.size).toBe(0) + expect(timer.metrics).toHaveLength(0) + }) + + test('_cleanupOrphanedSpan handles spans without timeouts', () => { + const timer = new PerformanceTimer({enabled: true}) + timer.mark('test', 'start') + + // Manually remove timeout to simulate edge case + timer.spanTimeouts.delete('test') + + expect(() => { + timer._cleanupOrphanedSpan('test', 'manual') + }).not.toThrow() + + expect(timer.spans.size).toBe(0) + }) + + test('_cleanupOrphanedSpan handles non-existent spans', () => { + const timer = new PerformanceTimer({enabled: true}) + + expect(() => { + timer._cleanupOrphanedSpan('nonexistent', 'manual') + }).not.toThrow() + }) + + test('handles createChildSpan returning null', () => { + const timer = new PerformanceTimer({enabled: true}) + + // Mock createChildSpan to return null + const {createChildSpan} = jest.requireMock('./opentelemetry') + createChildSpan.mockReturnValueOnce(null) + + timer.mark('test', 'start') + + expect(timer.spans.size).toBe(0) + expect(timer.spanTimeouts.size).toBe(0) + }) + + test('handles performance.clearMarks and performance.clearMeasures errors', () => { + const timer = new PerformanceTimer({enabled: true}) + + // Mock performance methods that might throw + const originalClearMarks = performance.clearMarks + const originalClearMeasures = performance.clearMeasures + + performance.clearMarks = jest.fn().mockImplementation(() => { + throw new Error('ClearMarks failed') + }) + performance.clearMeasures = jest.fn().mockImplementation(() => { + throw new Error('ClearMeasures failed') + }) + + timer.mark('test', 'start') + + expect(() => { + timer.mark('test', 'end') + }).not.toThrow() + + // Restore methods + performance.clearMarks = originalClearMarks + performance.clearMeasures = originalClearMeasures + }) + + test('handles span cleanup when performance.measure fails', () => { + const timer = new PerformanceTimer({enabled: true}) + + // Mock performance.measure to throw an error + const originalMeasure = performance.measure + performance.measure = jest.fn().mockImplementation(() => { + throw new Error('Measure failed') + }) + + timer.mark('test', 'start') + timer.mark('test', 'end') // This will fail but span remains + + expect(timer.spans.size).toBe(1) + + // Cleanup should remove the orphaned span + timer.cleanup() + + expect(timer.spans.size).toBe(0) + expect(timer.spanTimeouts.size).toBe(0) + + // Restore original method + performance.measure = originalMeasure + }) + }) + + describe('constructor options', () => { + test('accepts custom maxSpanDuration', () => { + const timer = new PerformanceTimer({enabled: true, maxSpanDuration: 5000}) + expect(timer.maxSpanDuration).toBe(5000) + }) + + test('uses default maxSpanDuration when not provided', () => { + const timer = new PerformanceTimer({enabled: true}) + expect(timer.maxSpanDuration).toBe(30000) + }) + + test('handles empty options object', () => { + const timer = new PerformanceTimer({}) + expect(timer.enabled).toBe(false) + expect(timer.maxSpanDuration).toBe(30000) + }) }) }) From 943a40c310d4cb5e009b5c398ef66bb32241d011 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 15:31:16 -0700 Subject: [PATCH 18/25] Remove handling of detail being an object --- packages/pwa-kit-react-sdk/src/utils/opentelemetry.js | 1 + packages/pwa-kit-react-sdk/src/utils/performance.js | 9 +++------ .../pwa-kit-react-sdk/src/utils/performance.test.js | 10 ---------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js index be8e269b7d..faaa188b00 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -24,6 +24,7 @@ const logSpanData = (span, event = 'start', res = null) => { const startTime = span.startTime const endTime = event === 'start' ? startTime : span.endTime const duration = event === 'start' ? 0 : hrTimeToMilliseconds(span.duration) + // Create the span data object that matches the expected format const spanData = { traceId: spanContext.traceId, diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index 68db711f63..ea955d6971 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -99,11 +99,8 @@ export default class PerformanceTimer { } try { - // Format detail as a string if it's an object - const formattedDetail = typeof detail === 'object' ? JSON.stringify(detail) : detail - performance.mark(`${name}.${type}`, { - detail: formattedDetail + detail: detail }) // Only create spans for 'start' events and store them for later use @@ -112,7 +109,7 @@ export default class PerformanceTimer { const span = createChildSpan(name, { performance_mark: name, performance_type: type, - performance_detail: formattedDetail + performance_detail: detail }) if (span) { this.spans.set(name, span) @@ -135,7 +132,7 @@ export default class PerformanceTimer { this.metrics.push({ name, duration: measure.duration, - detail: formattedDetail + detail: detail }) // End the corresponding span if it exists and clear timeout diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.test.js b/packages/pwa-kit-react-sdk/src/utils/performance.test.js index cc597036a5..b7760fe3a0 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.test.js @@ -73,16 +73,6 @@ describe('PerformanceTimer', () => { expect(timer.metrics[0].detail).toBe('end detail') }) - test('handles object detail correctly', () => { - const timer = new PerformanceTimer({enabled: true}) - const detailObj = {key: 'value', nested: {data: 123}} - timer.mark('test', 'start', {detail: detailObj}) - timer.mark('test', 'end', {detail: detailObj}) - - expect(timer.metrics).toHaveLength(1) - expect(timer.metrics[0].detail).toBe(JSON.stringify(detailObj)) - }) - test('buildServerTimingHeader returns empty string for no metrics', () => { const timer = new PerformanceTimer({enabled: true}) expect(timer.buildServerTimingHeader()).toBe('') From 40729666522e88084da1622bcce244382879653f Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 16:16:41 -0700 Subject: [PATCH 19/25] Some temporary? changes to get it to work --- packages/pwa-kit-react-sdk/setup-jest.js | 5 ----- .../src/ssr/server/react-rendering.js | 7 +++++++ .../src/utils/opentelemetry.js | 18 +++++++++++++----- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/pwa-kit-react-sdk/setup-jest.js b/packages/pwa-kit-react-sdk/setup-jest.js index e52cc2f789..7fe54bd88b 100644 --- a/packages/pwa-kit-react-sdk/setup-jest.js +++ b/packages/pwa-kit-react-sdk/setup-jest.js @@ -57,8 +57,3 @@ if (global.performance) { } else { global.performance = performance } - -// This is needed for spans created in jest environment to not be no-op spans, otherwise the spans will lack attributes -import {BasicTracerProvider} from '@opentelemetry/sdk-trace-base' -const provider = new BasicTracerProvider() -provider.register() 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 index 4d0b79b4c8..aece506645 100644 --- 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,12 @@ 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) { + // TODO: does this need to be synced with the config OTEL_SDK_ENABLED? + initializeServerTracing({enabled: true}) + } + res.__performanceTimer = new PerformanceTimer({enabled: shouldTrackPerformance}) res.__performanceTimer.mark(PERFORMANCE_MARKS.total, 'start') const AppConfig = getAppConfig() diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js index faaa188b00..8bd4de8392 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -12,9 +12,11 @@ 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' + serviceName: + (typeof process !== 'undefined' && process.env.OTEL_SERVICE_NAME) || DEFAULT_SERVICE_NAME, + enabled: typeof process !== 'undefined' && process.env.OTEL_SDK_ENABLED === 'true', + b3TracingEnabled: + typeof process !== 'undefined' && process.env.OTEL_B3_TRACING_ENABLED === 'true' } export const getServiceName = () => OTEL_CONFIG.serviceName @@ -22,6 +24,7 @@ export const getServiceName = () => OTEL_CONFIG.serviceName 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) @@ -47,8 +50,13 @@ const logSpanData = (span, event = 'start', res = null) => { forwardTrace: OTEL_CONFIG.b3TracingEnabled } - // Inject B3 headers into response if available - if (res && process.env.DISABLE_B3_TRACING !== 'true' && event === 'start') { + // Inject B3 headers into response if available (server-side only) + if ( + res && + typeof process !== 'undefined' && + process.env.DISABLE_B3_TRACING !== 'true' && + event === 'start' + ) { res.setHeader('x-b3-traceid', spanContext.traceId) res.setHeader('x-b3-spanid', spanContext.spanId) res.setHeader('x-b3-sampled', '1') From 6c5430d196ba84ab3c1b21474a3637b718247dd2 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 16:51:35 -0700 Subject: [PATCH 20/25] Update config --- .../src/ssr/server/opentelemetry-server.js | 4 +- .../src/utils/opentelemetry-config.js | 21 +++ .../src/utils/opentelemetry-config.test.js | 124 ++++++++++++++++++ .../src/utils/opentelemetry.js | 24 +--- .../src/utils/opentelemetry.test.js | 7 - 5 files changed, 151 insertions(+), 29 deletions(-) create mode 100644 packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.js create mode 100644 packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.test.js 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 index 51708d0182..4b9207c31b 100644 --- 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/utils/opentelemetry-config.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.js new file mode 100644 index 0000000000..196777c58a --- /dev/null +++ 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 new file mode 100644 index 0000000000..cc90c8a57d --- /dev/null +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.test.js @@ -0,0 +1,124 @@ +/* + * 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', () => { + test('should return default config when no environment variables are set', () => { + delete process.env.OTEL_SERVICE_NAME + delete process.env.OTEL_SDK_ENABLED + delete process.env.OTEL_B3_TRACING_ENABLED + + const {getOTELConfig} = mockModule() + const config = getOTELConfig() + + expect(config).toEqual({ + serviceName: 'pwa-kit-react-sdk', + enabled: false, + b3TracingEnabled: false + }) + }) + + test('should use environment variables when provided', () => { + process.env.OTEL_SERVICE_NAME = 'test-service' + process.env.OTEL_SDK_ENABLED = 'true' + process.env.OTEL_B3_TRACING_ENABLED = 'true' + + const {getOTELConfig} = mockModule() + const config = getOTELConfig() + + expect(config).toEqual({ + serviceName: 'test-service', + enabled: true, + b3TracingEnabled: true + }) + }) + + test('should handle partial environment variables', () => { + process.env.OTEL_SERVICE_NAME = 'custom-service' + process.env.OTEL_SDK_ENABLED = 'false' + delete process.env.OTEL_B3_TRACING_ENABLED + + const {getOTELConfig} = mockModule() + const config = getOTELConfig() + + expect(config).toEqual({ + serviceName: 'custom-service', + enabled: false, + b3TracingEnabled: false + }) + }) + + test('should treat non-"true" values as false for boolean flags', () => { + process.env.OTEL_SDK_ENABLED = 'false' + process.env.OTEL_B3_TRACING_ENABLED = 'yes' + + const {getOTELConfig} = mockModule() + const config = getOTELConfig() + + expect(config.enabled).toBe(false) + expect(config.b3TracingEnabled).toBe(false) + }) + + test('should handle empty string environment variables', () => { + process.env.OTEL_SERVICE_NAME = '' + process.env.OTEL_SDK_ENABLED = '' + process.env.OTEL_B3_TRACING_ENABLED = '' + + const {getOTELConfig} = mockModule() + const config = getOTELConfig() + + expect(config).toEqual({ + serviceName: 'pwa-kit-react-sdk', // Falls back to default + enabled: false, + b3TracingEnabled: false + }) + }) + }) + + describe('getServiceName', () => { + test('should return service name from config', () => { + process.env.OTEL_SERVICE_NAME = 'test-service-name' + + const {getServiceName} = mockModule() + const serviceName = getServiceName() + + expect(serviceName).toBe('test-service-name') + }) + + test('should return default service name when no env var set', () => { + delete process.env.OTEL_SERVICE_NAME + + const {getServiceName} = mockModule() + const serviceName = getServiceName() + + expect(serviceName).toBe('pwa-kit-react-sdk') + }) + }) +}) diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js index 8bd4de8392..381a23669c 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.js @@ -8,18 +8,7 @@ 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: - (typeof process !== 'undefined' && process.env.OTEL_SERVICE_NAME) || DEFAULT_SERVICE_NAME, - enabled: typeof process !== 'undefined' && process.env.OTEL_SDK_ENABLED === 'true', - b3TracingEnabled: - typeof process !== 'undefined' && 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() @@ -47,16 +36,11 @@ 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 (server-side only) - if ( - res && - typeof process !== 'undefined' && - process.env.DISABLE_B3_TRACING !== 'true' && - event === 'start' - ) { + // Inject B3 headers into response if available + if (res && process.env.DISABLE_B3_TRACING !== 'true' && event === 'start') { res.setHeader('x-b3-traceid', spanContext.traceId) res.setHeader('x-b3-spanid', spanContext.spanId) res.setHeader('x-b3-sampled', '1') diff --git a/packages/pwa-kit-react-sdk/src/utils/opentelemetry.test.js b/packages/pwa-kit-react-sdk/src/utils/opentelemetry.test.js index 0337eba7d7..7d17c4b294 100644 --- 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', { From 24b37e0c313a3a596c7863b12f3cda72dd7c8559 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 17:15:58 -0700 Subject: [PATCH 21/25] bundlesize increase --- packages/template-retail-react-app/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/template-retail-react-app/package.json b/packages/template-retail-react-app/package.json index 9ca5e072e2..9770d7d371 100644 --- a/packages/template-retail-react-app/package.json +++ b/packages/template-retail-react-app/package.json @@ -104,7 +104,7 @@ }, { "path": "build/vendor.js", - "maxSize": "328 kB" + "maxSize": "340 kB" } ] } From 6c301c516d98220b81a530a5db8d31b3754cfe62 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 23:05:46 -0700 Subject: [PATCH 22/25] Update CHANGELOG.md --- packages/template-retail-react-app/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/template-retail-react-app/CHANGELOG.md b/packages/template-retail-react-app/CHANGELOG.md index de2886a8b5..a8923b9e68 100644 --- a/packages/template-retail-react-app/CHANGELOG.md +++ b/packages/template-retail-react-app/CHANGELOG.md @@ -5,6 +5,7 @@ - Updated 6 new languages [#2495](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2495) - Fix Einstein event tracking for `addToCart` event [#2558](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2558) - Update latest translations for all languages [#2616](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2616) +- Bundlesize update [#2722](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2722) ## v6.1.0 (May 22, 2025) From 5a0f754950f73de1dc4f967b4388c4703246bc6e Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Thu, 10 Jul 2025 23:21:52 -0700 Subject: [PATCH 23/25] Update react-rendering.js --- packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 index aece506645..3a2041be80 100644 --- a/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js +++ b/packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js @@ -125,8 +125,7 @@ export const render = async (req, res, next) => { const shouldTrackPerformance = includeServerTimingHeader || process.env.SERVER_TIMING if (!isServerTracingInitialized() && shouldTrackPerformance) { - // TODO: does this need to be synced with the config OTEL_SDK_ENABLED? - initializeServerTracing({enabled: true}) + initializeServerTracing() } res.__performanceTimer = new PerformanceTimer({enabled: shouldTrackPerformance}) From e8dc896eb187d36a40563d181714bd6e2bc9d782 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Sun, 13 Jul 2025 23:30:05 -0700 Subject: [PATCH 24/25] Apply feedback --- .../src/utils/opentelemetry-config.test.js | 152 ++++++++++-------- 1 file changed, 87 insertions(+), 65 deletions(-) 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 index cc90c8a57d..1fc37daf98 100644 --- a/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.test.js +++ b/packages/pwa-kit-react-sdk/src/utils/opentelemetry-config.test.js @@ -30,95 +30,117 @@ describe('OpenTelemetry Config', () => { }) describe('getOTELConfig', () => { - test('should return default config when no environment variables are set', () => { - delete process.env.OTEL_SERVICE_NAME - delete process.env.OTEL_SDK_ENABLED - delete process.env.OTEL_B3_TRACING_ENABLED - - const {getOTELConfig} = mockModule() - const config = getOTELConfig() - - expect(config).toEqual({ - serviceName: 'pwa-kit-react-sdk', - enabled: false, - b3TracingEnabled: false + 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 use environment variables when provided', () => { - process.env.OTEL_SERVICE_NAME = 'test-service' - process.env.OTEL_SDK_ENABLED = 'true' - process.env.OTEL_B3_TRACING_ENABLED = 'true' + 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() + const {getOTELConfig} = mockModule() + const config = getOTELConfig() - expect(config).toEqual({ - serviceName: 'test-service', - enabled: true, - b3TracingEnabled: true + expect(config.serviceName).toBe('pwa-kit-react-sdk') }) - }) - test('should handle partial environment variables', () => { - process.env.OTEL_SERVICE_NAME = 'custom-service' - process.env.OTEL_SDK_ENABLED = 'false' - delete process.env.OTEL_B3_TRACING_ENABLED + test('should return default service name when OTEL_SERVICE_NAME is empty string', () => { + process.env.OTEL_SERVICE_NAME = '' - const {getOTELConfig} = mockModule() - const config = getOTELConfig() + const {getOTELConfig} = mockModule() + const config = getOTELConfig() - expect(config).toEqual({ - serviceName: 'custom-service', - enabled: false, - b3TracingEnabled: false + expect(config.serviceName).toBe('pwa-kit-react-sdk') }) }) - test('should treat non-"true" values as false for boolean flags', () => { - process.env.OTEL_SDK_ENABLED = 'false' - process.env.OTEL_B3_TRACING_ENABLED = 'yes' + describe('enabled', () => { + test('should return enabled when OTEL_SDK_ENABLED is "true"', () => { + process.env.OTEL_SDK_ENABLED = 'true' - const {getOTELConfig} = mockModule() - const config = getOTELConfig() + const {getOTELConfig} = mockModule() + const config = getOTELConfig() - expect(config.enabled).toBe(false) - expect(config.b3TracingEnabled).toBe(false) - }) + expect(config.enabled).toBe(true) + }) - test('should handle empty string environment variables', () => { - process.env.OTEL_SERVICE_NAME = '' - process.env.OTEL_SDK_ENABLED = '' - process.env.OTEL_B3_TRACING_ENABLED = '' + test('should return disabled when OTEL_SDK_ENABLED is not set', () => { + delete process.env.OTEL_SDK_ENABLED - const {getOTELConfig} = mockModule() - const config = getOTELConfig() + const {getOTELConfig} = mockModule() + const config = getOTELConfig() - expect(config).toEqual({ - serviceName: 'pwa-kit-react-sdk', // Falls back to default - enabled: false, - b3TracingEnabled: false + expect(config.enabled).toBe(false) }) - }) - }) - describe('getServiceName', () => { - test('should return service name from config', () => { - process.env.OTEL_SERVICE_NAME = 'test-service-name' + 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) + }) - const {getServiceName} = mockModule() - const serviceName = getServiceName() + test('should return disabled when OTEL_SDK_ENABLED is any non-"true" value', () => { + const nonTrueValues = ['yes', '1', 'True', 'TRUE', 'on', 'enabled', ''] - expect(serviceName).toBe('test-service-name') + nonTrueValues.forEach((value) => { + process.env.OTEL_SDK_ENABLED = value + + const {getOTELConfig} = mockModule() + const config = getOTELConfig() + + expect(config.enabled).toBe(false) + }) + }) }) - test('should return default service name when no env var set', () => { - delete process.env.OTEL_SERVICE_NAME + describe('b3TracingEnabled', () => { + test('should return enabled when OTEL_B3_TRACING_ENABLED is "true"', () => { + process.env.OTEL_B3_TRACING_ENABLED = 'true' - const {getServiceName} = mockModule() - const serviceName = getServiceName() + const {getOTELConfig} = mockModule() + const config = getOTELConfig() - expect(serviceName).toBe('pwa-kit-react-sdk') + 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) + }) + }) }) }) }) From 894549b0408c8bddda1532785f9e649d962aeaf4 Mon Sep 17 00:00:00 2001 From: Jang ho Jung Date: Wed, 16 Jul 2025 11:29:10 -0700 Subject: [PATCH 25/25] Apply feedback --- packages/pwa-kit-react-sdk/src/utils/performance.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/pwa-kit-react-sdk/src/utils/performance.js b/packages/pwa-kit-react-sdk/src/utils/performance.js index ea955d6971..b118b03688 100644 --- a/packages/pwa-kit-react-sdk/src/utils/performance.js +++ b/packages/pwa-kit-react-sdk/src/utils/performance.js @@ -89,7 +89,7 @@ export default class PerformanceTimer { */ mark(name, type, options = {}) { const {detail = ''} = options - if (!name || !type || !this.enabled) { + if (!this.enabled || !name || !type) { return } @@ -120,6 +120,8 @@ export default class PerformanceTimer { }, this.maxSpanDuration) this.spanTimeouts.set(name, timeoutId) } + } else { + logger.warn('Span already exists', {name, namespace: 'PerformanceTimer.mark'}) } } else if (type === this.MARKER_TYPES.END) { const startMark = `${name}.${this.MARKER_TYPES.START}` @@ -190,8 +192,8 @@ export default class PerformanceTimer { error: 'Deleting orphaned span (reason: ' + reason + ' cleanup)', namespace: 'PerformanceTimer._cleanupOrphanedSpan' }) - endSpan(span) this.spans.delete(name) + endSpan(span) } // Clear the timeout