Merge pull request #542 from vtex/fix/tracing-sampling-edge

Fix shouldTrace decision based on sampling decision from edge

CHANGELOG.md

@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+## [6.45.21] - 2023-09-19
+### Changed
+- Fix tracingMiddleware shouldTrace decision
+- Fix HttpClient tracing based on sampling decision
+
+## [6.45.21-beta.2] - 2023-09-19
+
+## [6.45.21-beta.1] - 2023-09-07
+
+## [6.45.21-beta.0] - 2023-09-04
+
+## [6.45.21-beta] - 2023-09-04
+
 ## [6.45.20] - 2023-08-30
 ### Changed
 - Remove sampling decision from runtime
@@ -1776,10 +1789,15 @@ instead
 - `HttpClient` now adds `'Accept-Encoding': 'gzip'` header by default.
 
 
-[Unreleased]: https://github.com/vtex/node-vtex-api/compare/v6.45.20...HEAD
+[Unreleased]: https://github.com/vtex/node-vtex-api/compare/v6.45.21...HEAD
 [6.45.15]: https://github.com/vtex/node-vtex-api/compare/v6.45.14...v6.45.15
 [6.45.14]: https://github.com/vtex/node-vtex-api/compare/v6.45.13...v6.45.14
 [6.45.13]: https://github.com/vtex/node-vtex-api/compare/v6.45.12...v6.45.13
 
+[6.45.21]: https://github.com/vtex/node-vtex-api/compare/v6.45.21-beta.2...v6.45.21
+[6.45.21-beta.2]: https://github.com/vtex/node-vtex-api/compare/v6.45.21-beta.1...v6.45.21-beta.2
+[6.45.21-beta.1]: https://github.com/vtex/node-vtex-api/compare/v6.45.21-beta.0...v6.45.21-beta.1
+[6.45.21-beta.0]: https://github.com/vtex/node-vtex-api/compare/v6.45.21-beta...v6.45.21-beta.0
+[6.45.21-beta]: https://github.com/vtex/node-vtex-api/compare/v6.45.20...v6.45.21-beta
 [6.45.20]: https://github.com/vtex/node-vtex-api/compare/v6.45.20-beta...v6.45.20
 [6.45.20-beta]: https://github.com/vtex/node-vtex-api/compare/v6.45.19...v6.45.20-beta

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@vtex/api",
-  "version": "6.45.20",
+  "version": "6.45.21",
   "description": "VTEX I/O API client",
   "main": "lib/index.js",
   "typings": "lib/index.d.ts",

src/HttpClient/middlewares/cache.ts

@@ -90,13 +90,13 @@ export const cacheMiddleware = ({ type, storage }: CacheOptions) => {
       return await next()
     }
 
-    const span = ctx.tracing!.rootSpan
+    const span = ctx.tracing?.rootSpan
 
     const key = cacheKey(ctx.config)
     const segmentToken = ctx.config.headers[SEGMENT_HEADER]
     const keyWithSegment = key + segmentToken
 
-    span.log({
+    span?.log({
       event: HttpLogEvents.CACHE_KEY_CREATE,
       [HttpCacheLogFields.CACHE_TYPE]: cacheType,
       [HttpCacheLogFields.KEY]: key,
@@ -115,7 +115,7 @@ export const cacheMiddleware = ({ type, storage }: CacheOptions) => {
 
       const now = Date.now()
 
-      span.log({
+      span?.log({
         event: HttpLogEvents.LOCAL_CACHE_HIT_INFO,
         [HttpCacheLogFields.CACHE_TYPE]: cacheType,
         [HttpCacheLogFields.ETAG]: cachedEtag,
@@ -132,18 +132,18 @@ export const cacheMiddleware = ({ type, storage }: CacheOptions) => {
           router: 0,
         }
 
-        span.setTag(CACHE_RESULT_TAG, CacheResult.HIT)
+        span?.setTag(CACHE_RESULT_TAG, CacheResult.HIT)
         return
       }
 
-      span.setTag(CACHE_RESULT_TAG, CacheResult.STALE)
+      span?.setTag(CACHE_RESULT_TAG, CacheResult.STALE)
       const validateStatus = addNotModified(ctx.config.validateStatus!)
       if (cachedEtag && validateStatus(response.status as number)) {
         ctx.config.headers['if-none-match'] = cachedEtag
         ctx.config.validateStatus = validateStatus
       }
     } else {
-      span.setTag(CACHE_RESULT_TAG, CacheResult.MISS)
+      span?.setTag(CACHE_RESULT_TAG, CacheResult.MISS)
     }
 
     await next()
@@ -168,7 +168,7 @@ export const cacheMiddleware = ({ type, storage }: CacheOptions) => {
     const {forceMaxAge} = ctx.config
     const maxAge = forceMaxAge && cacheableStatusCodes.includes(status) ? Math.max(forceMaxAge, headerMaxAge) : headerMaxAge
 
-    span.log({
+    span?.log({
       event: HttpLogEvents.CACHE_CONFIG,
       [HttpCacheLogFields.CACHE_TYPE]: cacheType,
       [HttpCacheLogFields.AGE]: age,
@@ -182,7 +182,7 @@ export const cacheMiddleware = ({ type, storage }: CacheOptions) => {
 
     // Indicates this should NOT be cached and this request will not be considered a miss.
     if (!forceMaxAge && (noStore || (noCache && !etag))) {
-      span.log({ event: HttpLogEvents.NO_LOCAL_CACHE_SAVE, [HttpCacheLogFields.CACHE_TYPE]: cacheType })
+      span?.log({ event: HttpLogEvents.NO_LOCAL_CACHE_SAVE, [HttpCacheLogFields.CACHE_TYPE]: cacheType })
       return
     }
 
@@ -207,7 +207,7 @@ export const cacheMiddleware = ({ type, storage }: CacheOptions) => {
         responseType,
       })
 
-      span.log({
+      span?.log({
         event: HttpLogEvents.LOCAL_CACHE_SAVED,
         [HttpCacheLogFields.CACHE_TYPE]: cacheType,
         [HttpCacheLogFields.KEY_SET]: setKey,
@@ -221,7 +221,7 @@ export const cacheMiddleware = ({ type, storage }: CacheOptions) => {
       return
     }
 
-    span.log({ event: HttpLogEvents.NO_LOCAL_CACHE_SAVE, [HttpCacheLogFields.CACHE_TYPE]: cacheType })
+    span?.log({ event: HttpLogEvents.NO_LOCAL_CACHE_SAVE, [HttpCacheLogFields.CACHE_TYPE]: cacheType })
   }
 }
 

src/HttpClient/middlewares/memoization.ts

@@ -17,21 +17,21 @@ export const memoizationMiddleware = ({ memoizedCache }: MemoizationOptions) =>
       return next()
     }
 
-    const span = ctx.tracing!.rootSpan
+    const span = ctx.tracing?.rootSpan
 
     const key = cacheKey(ctx.config)
     const isMemoized = !!memoizedCache.has(key)
 
-    span.log({ event: HttpLogEvents.CACHE_KEY_CREATE, [HttpCacheLogFields.CACHE_TYPE]: 'memoization', [HttpCacheLogFields.KEY]: key })
+    span?.log({ event: HttpLogEvents.CACHE_KEY_CREATE, [HttpCacheLogFields.CACHE_TYPE]: 'memoization', [HttpCacheLogFields.KEY]: key })
 
     if (isMemoized) {
-      span.setTag(CustomHttpTags.HTTP_MEMOIZATION_CACHE_RESULT, CacheResult.HIT)
+      span?.setTag(CustomHttpTags.HTTP_MEMOIZATION_CACHE_RESULT, CacheResult.HIT)
       const memoized = await memoizedCache.get(key)!
       ctx.memoizedHit = isMemoized
       ctx.response = memoized.response
       return
     } else {
-      span.setTag(CustomHttpTags.HTTP_MEMOIZATION_CACHE_RESULT, CacheResult.MISS)
+      span?.setTag(CustomHttpTags.HTTP_MEMOIZATION_CACHE_RESULT, CacheResult.MISS)
       const promise = new Promise<Memoized>(async (resolve, reject) => {
         try {
           await next()
@@ -40,10 +40,10 @@ export const memoizationMiddleware = ({ memoizedCache }: MemoizationOptions) =>
             response: ctx.response!,
           })
 
-          span.log({ event: HttpLogEvents.MEMOIZATION_CACHE_SAVED, [HttpCacheLogFields.KEY_SET]: key })
+          span?.log({ event: HttpLogEvents.MEMOIZATION_CACHE_SAVED, [HttpCacheLogFields.KEY_SET]: key })
         } catch (err) {
           reject(err)
-          span.log({ event: HttpLogEvents.MEMOIZATION_CACHE_SAVED_ERROR, [HttpCacheLogFields.KEY_SET]: key })
+          span?.log({ event: HttpLogEvents.MEMOIZATION_CACHE_SAVED_ERROR, [HttpCacheLogFields.KEY_SET]: key })
         }
       })
       memoizedCache.set(key, promise)

src/HttpClient/middlewares/request/index.ts

@@ -65,10 +65,10 @@ export const defaultsMiddleware = ({ baseURL, rawHeaders, params, timeout, retri
     }
 
 
-    if(ctx.tracing!.isSampled) {
+    if(ctx.tracing?.isSampled) {
       const { config } = ctx
       const fullUrl = buildFullPath(config.baseURL, http.getUri(config))
-      ctx.tracing!.rootSpan.addTags({
+      ctx.tracing?.rootSpan?.addTags({
         [OpentracingTags.HTTP_METHOD]: config.method || 'get',
         [OpentracingTags.HTTP_URL]: fullUrl,
       })
@@ -89,7 +89,7 @@ export const routerCacheMiddleware = async (ctx: MiddlewareContext, next: () =>
   const status = ctx.response?.status
 
   if(routerCacheHit) {
-    ctx.tracing!.rootSpan.setTag(CustomHttpTags.HTTP_ROUTER_CACHE_RESULT, routerCacheHit)
+    ctx.tracing?.rootSpan?.setTag(CustomHttpTags.HTTP_ROUTER_CACHE_RESULT, routerCacheHit)
   }
 
   if (routerCacheHit === ROUTER_CACHE_HIT || (routerCacheHit === ROUTER_CACHE_REVALIDATED && status !== 304)) {
@@ -106,4 +106,4 @@ export const requestMiddleware = (limit?: Limit) => async (ctx: MiddlewareContex
   const makeRequest = () => http.request(ctx.config)
 
   ctx.response = await (limit ? limit(makeRequest) : makeRequest())
-}
+}

src/HttpClient/middlewares/request/setupAxios/interceptors/exponentialBackoff.ts

@@ -65,7 +65,7 @@ const onResponseError = (http: AxiosInstance) => (error: any) => {
       : config.timeout
     config.transformRequest = [data => data]
 
-    config.tracing!.rootSpan!.log({ event: HttpLogEvents.SETUP_REQUEST_RETRY, [HttpRetryLogFields.RETRY_NUMBER]: config.retryCount, [HttpRetryLogFields.RETRY_IN]: delay })
+    config.tracing?.rootSpan?.log({ event: HttpLogEvents.SETUP_REQUEST_RETRY, [HttpRetryLogFields.RETRY_NUMBER]: config.retryCount, [HttpRetryLogFields.RETRY_IN]: delay })
 
     return new Promise(resolve => setTimeout(() => resolve(http(config)), delay))
   }

src/HttpClient/middlewares/request/setupAxios/interceptors/tracing/index.ts

@@ -26,7 +26,7 @@ export const requestSpanPrefix = 'http-request'
 const preRequestInterceptor = (http: AxiosInstance) => (
   config: TraceableAxiosRequestConfig
 ): TraceableAxiosRequestConfig => {
-  if (!config.tracing || !config.tracing.isSampled) {
+  if (!config.tracing || !config.tracing?.isSampled) {
     return config
   }
 
@@ -48,18 +48,18 @@ const preRequestInterceptor = (http: AxiosInstance) => (
 }
 
 const onResponseSuccess = (response: TraceableAxiosResponse): TraceableAxiosResponse => {
-  if (!response.config.tracing || !response.config.tracing.isSampled) {
+  if (!response.config.tracing || !response.config.tracing?.isSampled) {
     return response
   }
 
-  const requestSpan = response.config.tracing.requestSpan!
+  const requestSpan = response.config.tracing?.requestSpan
   injectResponseInfoOnSpan(requestSpan, response)
-  requestSpan.finish()
+  requestSpan?.finish()
   return response
 }
 
 const onResponseError = (err: ExtendedAxiosError) => {
-  if (!err?.config?.tracing?.requestSpan || !err.config.tracing.isSampled) {
+  if (!err?.config?.tracing?.requestSpan || !err.config.tracing?.isSampled) {
     return Promise.reject(err)
   }
 

src/HttpClient/middlewares/request/setupAxios/interceptors/tracing/spanSetup.ts

@@ -5,26 +5,26 @@ import { ROUTER_CACHE_HEADER } from '../../../../../../constants'
 import { CustomHttpTags, OpentracingTags } from '../../../../../../tracing/Tags'
 import { cloneAndSanitizeHeaders } from '../../../../../../tracing/utils'
 
-export const injectRequestInfoOnSpan = (span: Span, http: AxiosInstance, config: AxiosRequestConfig) => {
-  span.addTags({
+export const injectRequestInfoOnSpan = (span: Span | undefined, http: AxiosInstance, config: AxiosRequestConfig) => {
+  span?.addTags({
     [OpentracingTags.SPAN_KIND]: OpentracingTags.SPAN_KIND_RPC_CLIENT,
     [OpentracingTags.HTTP_METHOD]: config.method,
     [OpentracingTags.HTTP_URL]: buildFullPath(config.baseURL, http.getUri(config)),
   })
 
-  span.log({ 'request-headers': cloneAndSanitizeHeaders(config.headers) })
+  span?.log({ 'request-headers': cloneAndSanitizeHeaders(config.headers) })
 }
 
 // Response may be undefined in case of client timeout, invalid URL, ...
-export const injectResponseInfoOnSpan = (span: Span, response: AxiosResponse<any> | undefined) => {
+export const injectResponseInfoOnSpan = (span: Span | undefined, response: AxiosResponse<any> | undefined) => {
   if (!response) {
-    span.setTag(CustomHttpTags.HTTP_NO_RESPONSE, 'true')
+    span?.setTag(CustomHttpTags.HTTP_NO_RESPONSE, 'true')
     return
   }
 
-  span.log({ 'response-headers': cloneAndSanitizeHeaders(response.headers) })
-  span.setTag(OpentracingTags.HTTP_STATUS_CODE, response.status)
+  span?.log({ 'response-headers': cloneAndSanitizeHeaders(response.headers) })
+  span?.setTag(OpentracingTags.HTTP_STATUS_CODE, response.status)
   if (response.headers[ROUTER_CACHE_HEADER]) {
-    span.setTag(CustomHttpTags.HTTP_ROUTER_CACHE_RESULT, response.headers[ROUTER_CACHE_HEADER])
+    span?.setTag(CustomHttpTags.HTTP_ROUTER_CACHE_RESULT, response.headers[ROUTER_CACHE_HEADER])
   }
 }

src/HttpClient/middlewares/tracing.ts

@@ -1,6 +1,6 @@
 import { MiddlewaresTracingContext, RequestConfig } from '..'
 import { IOContext } from '../../service/worker/runtime/typings'
-import { createSpanReference, ErrorReport, getTraceInfo, SpanReferenceTypes } from '../../tracing'
+import { ErrorReport, getTraceInfo } from '../../tracing'
 import { CustomHttpTags, OpentracingTags } from '../../tracing/Tags'
 import { MiddlewareContext } from '../typings'
 import { CacheType, isLocallyCacheable } from './cache'
@@ -25,13 +25,15 @@ export const createHttpClientTracingMiddleware = ({
   hasDiskCacheMiddleware,
 }: HttpClientTracingMiddlewareConfig) => {
   return async function tracingMiddleware(ctx: MiddlewareContext, next: () => Promise<void>) {
-    const { rootSpan, requestSpanNameSuffix, referenceType = SpanReferenceTypes.CHILD_OF } = ctx.config.tracing || {}
+    if(!tracer.isTraceSampled){
+      await next()
+      return
+    }
+
+    const rootSpan = tracer.fallbackSpanContext()
+    const { requestSpanNameSuffix } = ctx.config.tracing || {}
     const spanName = requestSpanNameSuffix ? `request:${requestSpanNameSuffix}` : 'request'
-    const span = rootSpan
-      ? tracer.startSpan(spanName, {
-          references: [createSpanReference(rootSpan, referenceType)],
-        })
-      : tracer.startSpan(spanName)
+    const span = tracer.startSpan(spanName, {childOf: rootSpan})
 
     ctx.tracing = {
       ...ctx.config.tracing,
@@ -49,7 +51,7 @@ export const createHttpClientTracingMiddleware = ({
     const hasMemoryCache = hasMemoryCacheMiddleware && !!isLocallyCacheable(ctx.config, CacheType.Memory)
     const hasDiskCache = hasDiskCacheMiddleware && !!isLocallyCacheable(ctx.config, CacheType.Disk)
 
-    span.addTags({
+    span?.addTags({
       [CustomHttpTags.HTTP_MEMOIZATION_CACHE_ENABLED]: hasMemoCache,
       [CustomHttpTags.HTTP_MEMORY_CACHE_ENABLED]: hasMemoryCache,
       [CustomHttpTags.HTTP_DISK_CACHE_ENABLED]: hasDiskCache,
@@ -62,19 +64,19 @@ export const createHttpClientTracingMiddleware = ({
       response = ctx.response
     } catch (err) {
       response = err.response
-      if(ctx.tracing.isSampled) {
+      if(ctx.tracing?.isSampled) {
         ErrorReport.create({ originalError: err }).injectOnSpan(span, logger)
       }
-      
+
       throw err
     } finally {
       if (response) {
-        span.setTag(OpentracingTags.HTTP_STATUS_CODE, response.status)
+        span?.setTag(OpentracingTags.HTTP_STATUS_CODE, response.status)
       } else {
-        span.setTag(CustomHttpTags.HTTP_NO_RESPONSE, true)
+        span?.setTag(CustomHttpTags.HTTP_NO_RESPONSE, true)
       }
 
-      span.finish()
+      span?.finish()
     }
   }
 }

src/HttpClient/typings.ts

@@ -63,7 +63,7 @@ export interface CacheHit {
 export interface MiddlewaresTracingContext extends Omit<RequestTracingUserConfig, 'rootSpan'> {
   tracer: IUserLandTracer
   logger: IOContext['logger']
-  rootSpan: Span
+  rootSpan?: Span
   isSampled: boolean
 }
 

src/service/logger/logger.ts

@@ -26,7 +26,7 @@ interface LoggerContext extends  Pick<IOContext, 'account'|'workspace'|'requestI
 
 interface TracingState {
   isTraceSampled: boolean,
-  traceId: string
+  traceId?: string
 }
 
 export class Logger {