@W-18759774 - Created opentelemetry.js file with utility functions to log OTel spans and metrics by larnelleankunda · Pull Request #2705 · SalesforceCommerceCloud/pwa-kit

larnelleankunda · 2025-07-02T00:26:30Z

Created opentelemetry.js file with utility functions to log OTel spans and metrics as well as creates a test file for it

Description

Types of Changes

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Documentation update
Breaking change (could cause existing functionality to not work as expected)
Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

Removing a public function or component or prop

Adding a required argument to a function

Changing the data type of a function parameter or return value

Adding a new peer dependency to package.json

Changes

Added an open telemetry javascript file with utility functions to log OTel Spans and Metrics as well as created tests for it.

How to Test-Drive This PR

Within the pwa-kit-react-sdk folder run this command to see if all tests for the open telemetry javascript file with utility functions pass - npm run test pwa-kit/packages/pwa-kit-react-sdk/src/utils/opentelemetry.test.js

Checklists

General

Changes are covered by test cases
CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

There are no changes to UI

or...

Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
Changes comply with WCAG 2.0 guidelines levels A and AA
Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

Changes include a UI text update in the Retail React App (which requires translation)

…s and metricsas well as creates a test file for it

cc-prodsec · 2025-07-02T00:26:49Z

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

Signed-off-by: Larnelle Ankunda <lankunda@salesforce.com>

adamraya · 2025-07-04T22:31:47Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+import {hrTimeToMilliseconds, hrTimeToTimeStamp} from '@opentelemetry/core'
+import logger from './logger-instance'
+
+const SERVICE_NAME = 'pwa-kit-react-sdk'


We have now have two different places where we are defining the service name value, one in the /utils/opentelemetry.js file and another one in the /server/opentelemetry-server.js file:

pwa-kit/packages/pwa-kit-react-sdk/src/ssr/server/opentelemetry-server.js

Line 15 in 9130b9a

const DEFAULT_SERVICE_NAME = 'pwa-kit-react-sdk'

What do you think if we keep the opentelemetry config in the /utils/opentelemetry.js file as the source of truth, so we consistently use the same config values in the opentelemetry utils and opetelemetry server initialization files?

Example:

//utils/opentelemetry.js const DEFAULT_SERVICE_NAME = 'pwa-kit-react-sdk' export const OTEL_CONFIG = { serviceName: process.env.OTEL_SERVICE_NAME || DEFAULT_SERVICE_NAME, enabled: process.env.OTEL_SDK_ENABLED === 'true', b3TracingEnabled: process.env.OTEL_B3_TRACING_ENABLED === 'true', } export const getServiceName = () => OTEL_CONFIG.serviceName

Re-use the config in the opentelemetry-server.js file:

//server/opentelemetry-server.js import {OTEL_CONFIG} from '../../utils/opentelemetry' export const initializeServerTracing = (options = {}) => { const config = { ...OTEL_CONFIG, ...options } ... }

Later, in future PRs we can use the config like this:

//server/react-rendering.js import {OTEL_CONFIG} from '../../utils/opentelemetry' import {initializeServerTracing} from './opentelemetry-server' initializeServerTracing(OTEL_CONFIG) ...

As I actually use the opentelemetry.js in peformance.js in development which gets it closer to working end-to-end. I see that the app errors out with an uncaught exception that process is not defined.

I believe this is because process.env, when accessed from this config in the top-level portion of this file outside of any functions, is in the context of the browser environment where it doesn't have the scope for process. Webpack includes the opentelemetry.js in the app bundle and the browser executes the top-level code. We might want to keep any usage of process.env within functions that are called server-side.

I'll make some changes to this in my performance.js PR

adamraya · 2025-07-04T22:33:47Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+        timestamp: hrTimeToTimeStamp(startTime),
+        duration: duration,
+        attributes: {
+            'service.name': SERVICE_NAME,


If you agree with the previous comment we need to replace all references of SERVICE_NAME:

Suggested change

'service.name': SERVICE_NAME,

'service.name': getServiceName(),

adamraya · 2025-07-04T22:37:11Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+        links: [],
+        start_time: startTime,
+        end_time: endTime,
+        forwardTrace: process.env.DISABLE_B3_TRACING !== 'true'


Suggested change

forwardTrace: process.env.DISABLE_B3_TRACING !== 'true'

forwardTrace: OTEL_CONFIG.b3TracingEnabled

adamraya · 2025-07-04T22:43:44Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+
+const SERVICE_NAME = 'pwa-kit-react-sdk'
+
+function logSpanData(span, event = 'start', res = null) {


Can we use the arrow function pattern used throughout the codebase instead of regular function for consistency?

Suggested change

function logSpanData(span, event = 'start', res = null) {

const logSpanData = (span, event = 'start', res = null) => {

yunakim714 · 2025-07-09T14:53:16Z

packages/pwa-kit-react-sdk/jest.config.js

 * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
 */
 /* eslint-disable @typescript-eslint/no-var-requires */
-const base = require('internal-lib-build/configs/jest/jest.config')


Nit: Do we need this change?

+1 on this nit. It may not seem like a big deal that a change like this that doesn't do anything goes in the PR, but one downside is in the future if someone wants to do a git blame to see "hey who added this line about requiring jest.config here and why did they do that" then they will see this PR instead of the originating PR.

yunakim714 · 2025-07-09T14:55:31Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+
+export const OTEL_CONFIG = {
+    serviceName: process.env.OTEL_SERVICE_NAME || DEFAULT_SERVICE_NAME,
+    enabled: process.env.OTEL_SDK_ENABLED === 'true',


I see we have an enabled flag in the CONFIG but are we reading this in anywhere? Or will this be used in future PR's when we make changes to performance and react-rendering?

yunakim714 · 2025-07-09T14:57:30Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+
+export const getServiceName = () => OTEL_CONFIG.serviceName
+
+const SERVICE_NAME = OTEL_CONFIG.serviceName


Suggested change

const SERVICE_NAME = OTEL_CONFIG.serviceName

We don't need this line. As Adam mentioned, we can replace all instances of SERVICE_NAME with getServiceName()

yunakim714 · 2025-07-09T15:02:00Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+ */
+export const createSpan = (name, options = {}) => {
+    try {
+        const tracer = trace.getTracer(SERVICE_NAME)


For example, we would replace it here:

Suggested change

const tracer = trace.getTracer(SERVICE_NAME)

const tracer = trace.getTracer(getServiceName())

yunakim714 · 2025-07-09T15:11:01Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+        const ctx = context.active()
+        // Note: parentSpan is not used in this implementation


Do we need these 2 lines if ctx is not used anywhere?

yunakim714 · 2025-07-09T15:11:58Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.test.js

+/*
+ * 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
+ */


Duplicate

Suggested change

/*

* 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

*/

adamraya

Thanks for making the changes. LGTM

adamraya · 2025-07-10T00:00:13Z

packages/pwa-kit-react-sdk/src/utils/opentelemetry.js

+
+const DEFAULT_SERVICE_NAME = 'pwa-kit-react-sdk'
+
+export const OTEL_CONFIG = {


Let's use the OTEL_CONFIG in packages/pwa-kit-react-sdk/src/utils/opentelemetry-server.js file.

Remove the DEFAULT_SERVICE_NAME constant from opentelemetry-server.js:

pwa-kit/packages/pwa-kit-react-sdk/src/ssr/server/opentelemetry-server.js

Line 15 in 01da556

const DEFAULT_SERVICE_NAME = 'pwa-kit-react-sdk'

And replace it with getServiceName:

import {getServiceName, OTEL_CONFIG} from '../../utils/opentelemetry' const serviceName = options.serviceName || getServiceName() ...

Created opentelemetry.js file with utility functions to log OTel span…

17a3b2b

…s and metricsas well as creates a test file for it

larnelleankunda requested a review from a team as a code owner July 2, 2025 00:26

larnelleankunda added 2 commits July 1, 2025 17:27

Updated change log file

cae20be

Merge branch 'feature/opentelemetry' into W-18759774-opentelemetry-util

682baf4

Signed-off-by: Larnelle Ankunda <lankunda@salesforce.com>

larnelleankunda changed the title ~~Created opentelemetry.js file with utility functions to log OTel spans and metrics~~ @W-18759774 - Created opentelemetry.js file with utility functions to log OTel spans and metrics Jul 2, 2025

larnelleankunda added 3 commits July 2, 2025 09:33

Linted utility and test files

abbcf6d

Linted utility and test files

c13f041

updated utility tests

d3124b7

adamraya reviewed Jul 4, 2025

View reviewed changes

larnelleankunda added 5 commits July 7, 2025 16:20

updated utility functions for open telemetry

ea66f26

updated utility functions for open telemetry

1afef73

brought back open telemetry server files

9a1e0ec

brought back open telemetry server files

2c54543

brought back open telemetry server files

01da556

yunakim714 reviewed Jul 9, 2025

View reviewed changes

edited open telemetry js and test file

f6ffadf

adamraya approved these changes Jul 10, 2025

View reviewed changes

edited open telemetry server and test file

0a83204

jeremy-jung1 mentioned this pull request Jul 10, 2025

@W-18760780 Integrate opentelemetry into performance.js #2722

Merged

12 tasks

larnelleankunda added 2 commits July 11, 2025 16:16

reverting jest config changes

d344dc9

linted files

c68195a

jeremy-jung1 approved these changes Jul 11, 2025

View reviewed changes

larnelleankunda merged commit 4fa34a2 into feature/opentelemetry Jul 11, 2025
35 checks passed

jeremy-jung1 mentioned this pull request Aug 20, 2025

@W-18900367 Integration of opentelemetry for SSR Tracing #3133

Merged

12 tasks

	'service.name': SERVICE_NAME,
	'service.name': getServiceName(),

	forwardTrace: process.env.DISABLE_B3_TRACING !== 'true'
	forwardTrace: OTEL_CONFIG.b3TracingEnabled


		const SERVICE_NAME = 'pwa-kit-react-sdk'

		function logSpanData(span, event = 'start', res = null) {

	function logSpanData(span, event = 'start', res = null) {
	const logSpanData = (span, event = 'start', res = null) => {


		export const getServiceName = () => OTEL_CONFIG.serviceName

		const SERVICE_NAME = OTEL_CONFIG.serviceName

	const tracer = trace.getTracer(SERVICE_NAME)
	const tracer = trace.getTracer(getServiceName())

		const ctx = context.active()
		// Note: parentSpan is not used in this implementation


		const DEFAULT_SERVICE_NAME = 'pwa-kit-react-sdk'

		export const OTEL_CONFIG = {

Conversation

larnelleankunda commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of Changes

Changes

How to Test-Drive This PR

Checklists

General

Accessibility Compliance

Localization

Uh oh!

cc-prodsec commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎉 Snyk checks have passed. No issues have been found so far.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremy-jung1 Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamraya Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamraya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

larnelleankunda commented Jul 2, 2025 •

edited

Loading

cc-prodsec commented Jul 2, 2025 •

edited

Loading

jeremy-jung1 Jul 10, 2025 •

edited

Loading

adamraya Jul 4, 2025 •

edited

Loading