@W-18759755 - Creating tests and a server for open telemetry within the pwa-kit-rea…#2617
Conversation
…ct-sdk folder acting as a service provider
🎉 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) |
…ct-sdk folder acting as a service provider
…ith open telemetry dependencies
|
@larnelleankunda Hey Larnelle! I would create a feature branch that we can merge into, so that we are not merging an incomplete feature into the Could you also update the PR description - I see this at the top not sure if it cut something off! "…ct-sdk folder acting as a service provider" |
| @@ -0,0 +1,74 @@ | |||
| /* | |||
| * Copyright (c) 2024, Salesforce, Inc. | |||
There was a problem hiding this comment.
| * Copyright (c) 2024, Salesforce, Inc. | |
| * Copyright (c) 2025, Salesforce, Inc. |
| "@loadable/server": "^5.15.3", | ||
| "@loadable/webpack-plugin": "^5.15.2", | ||
| "@opentelemetry/api": "^1.4.1", | ||
| "@opentelemetry/core": "^1.15.1", |
There was a problem hiding this comment.
Do we need to include the @opentelemetry/core package here? We are not importing anything from this package in the new opentelemetry-server.js file
There was a problem hiding this comment.
I was migrating all the packages from the POC branch into this package.json but I can remove the core package.
| * @jest-environment node | ||
| */ | ||
| /* | ||
| * Copyright (c) 2024, Salesforce, Inc. |
There was a problem hiding this comment.
| * Copyright (c) 2024, Salesforce, Inc. | |
| * Copyright (c) 2025, Salesforce, Inc. |
| * Initialize OpenTelemetry tracing for server-side rendering | ||
| * @returns {NodeTracerProvider|null} The initialized provider or null if initialization failed | ||
| */ | ||
| export const initializeServerTracing = () => { |
There was a problem hiding this comment.
Would you consider making the telemetry configurable by passing options to the initializeServerTracing?
Something like this:
| export const initializeServerTracing = () => { | |
| export const initializeServerTracing = (options={}) => { | |
| const { | |
| serviceName = process.env.OTEL_SERVICE_NAME || DEFAULT_SERVICE_NAME, | |
| serviceVersion = process.env.npm_package_version, | |
| enabled = process.env.OTEL_SDK_ENABLED=== 'true' | |
| } = options | |
| ... |
| return provider | ||
| } catch (error) { | ||
| // Log errors from OpenTelemetry initialization | ||
| console.warn('Failed to initialize OpenTelemetry provider:', error.message) |
There was a problem hiding this comment.
Let's emit the log warnings using the PWAKitLogger to add a namespace.
E.g.
| await provider.shutdown() | ||
| provider = null // Clean up after successful shutdown | ||
| } catch (error) { | ||
| console.warn('Failed to shutdown OpenTelemetry provider:', error.message) |
There was a problem hiding this comment.
Same, let's use logger instead of directly using console.
| // Initialize the tracer provider | ||
| provider = new NodeTracerProvider({ | ||
| resource: new Resource({ | ||
| [SemanticResourceAttributes.SERVICE_NAME]: SERVICE_NAME |
There was a problem hiding this comment.
Did you look into the deprecation warnings?
It looks like SemanticResourceAttributes is deprecated an we should instead SEMRESATTRS_SERVICE_NAME
| export const initializeServerTracing = (options = {}) => { | ||
| const { | ||
| serviceName = process.env.OTEL_SERVICE_NAME || DEFAULT_SERVICE_NAME, | ||
| serviceVersion = process.env.npm_package_version, |
There was a problem hiding this comment.
@larnelleankunda Thanks for adding the telemetry options! It was my bad for not clarifying the code I shared was just an example of possible configuration options. Since process.env.npm_package_version will always be undefined, let’s remove it for now. We can explore a better way to include it later if needed.
| serviceVersion = process.env.npm_package_version, |
Creating tests and a server for open telemetry within the pwa-kit-react-sdk folder acting as a service provider
Description
<!
Implemented the functions to initialize and shutdown OTel tracing
Implemented
initializeServerTracingandshutdownServerTracingfunctions into the open telemetry serverB3 propagation is properly set up for tracing
Error handling is implemented to catch and log failures
Wrote unit tests with 100% coverage of new code>
Types of Changes
Changes
How to Test-Drive This PR
-To verify B3 Propagation you have make a request to the PWA Kit app locally.
b3 header (single header format)
Or X-B3-SpanId, X-B3-TracenId, X-B3-Sampled
-The B3 headers should be present in responses
-In Verifying its Shutdown you should stop the application and no OpenTelemetry-related errors during shutdown should be seen.
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization