fix: optional debug telemetry (#566)

supabase · Oct 8, 2024 · c8f93e5 · c8f93e5
1 parent d4a0cd8
commit c8f93e5
Show file tree

Hide file tree

Showing 5 changed files with 170 additions and 109 deletions.
diff --git a/src/config.ts b/src/config.ts
@@ -117,6 +117,9 @@ type StorageConfigType = {
   tracingMode?: string
   tracingTimeMinDuration: number
   tracingReturnServerTimings: boolean
+  tracingFeatures?: {
+    upload: boolean
+  }
 }
 
 function getOptionalConfigFromEnv(key: string, fallback?: string): string | undefined {
@@ -326,6 +329,9 @@ export function getConfig(options?: { reload?: boolean }): StorageConfigType {
     ),
     tracingReturnServerTimings:
       getOptionalConfigFromEnv('TRACING_RETURN_SERVER_TIMINGS') === 'true',
+    tracingFeatures: {
+      upload: getOptionalConfigFromEnv('TRACING_FEATURE_UPLOAD') === 'true',
+    },
 
     // Queue
     pgQueueEnable: getOptionalConfigFromEnv('PG_QUEUE_ENABLE', 'ENABLE_QUEUE_EVENTS') === 'true',

diff --git a/src/http/plugins/tracing.ts b/src/http/plugins/tracing.ts
@@ -24,6 +24,8 @@ const {
   tracingTimeMinDuration,
 } = getConfig()
 
+const enableLogTraces = ['debug', 'logs'].includes(defaultTracingMode || '')
+
 export const tracing = fastifyPlugin(
   async function tracingMode(fastify) {
     if (!tracingEnabled) {
@@ -40,15 +42,15 @@ export const tracing = fastifyPlugin(
           request.tracingMode = defaultTracingMode
         }
 
+        if (!enableLogTraces) {
+          return
+        }
+
         const span = trace.getSpan(context.active())
 
         if (span) {
           // We collect logs only in full,logs,debug mode
-          if (
-            tracingEnabled &&
-            request.tracingMode &&
-            !['logs', 'debug'].includes(request.tracingMode)
-          ) {
+          if (request.tracingMode && !['debug'].includes(request.tracingMode)) {
             traceCollector.clearTrace(span.spanContext().traceId)
           }
         }
@@ -62,7 +64,7 @@ export const tracing = fastifyPlugin(
 
 export const traceServerTime = fastifyPlugin(
   async function traceServerTime(fastify) {
-    if (!tracingEnabled) {
+    if (!tracingEnabled || !enableLogTraces) {
       return
     }
     fastify.addHook('onRequest', async (req, res) => {
@@ -94,9 +96,14 @@ export const traceServerTime = fastifyPlugin(
     })
 
     fastify.addHook('onResponse', async (request, reply) => {
-      try {
-        const traceId = trace.getSpan(context.active())?.spanContext().traceId
+      const traceId = trace.getSpan(context.active())?.spanContext().traceId
 
+      if (request.tracingMode !== 'debug') {
+        if (traceId) traceCollector.clearTrace(traceId)
+        return
+      }
+
+      try {
         if (traceId) {
           const spans = traceCollector.getSpansForTrace(traceId)
           if (spans) {
@@ -116,30 +123,41 @@ export const traceServerTime = fastifyPlugin(
                 .join(',')
               reply.header('Server-Timing', httpServerTimes)
             }
-            traceCollector.clearTrace(traceId)
           }
         }
       } catch (e) {
         logSchema.error(request.log, 'failed tracing on response', { error: e, type: 'tracing' })
+      } finally {
+        if (traceId) {
+          traceCollector.clearTrace(traceId)
+        }
       }
     })
 
     fastify.addHook('onRequestAbort', async (req) => {
-      try {
-        const span = trace.getSpan(context.active())
-        const traceId = span?.spanContext().traceId
+      const span = trace.getSpan(context.active())
+      const traceId = span?.spanContext().traceId
+
+      if (req.tracingMode !== 'debug') {
+        if (traceId) traceCollector.clearTrace(traceId)
+        return
+      }
 
+      try {
         span?.setAttribute('req_aborted', true)
 
         if (traceId) {
           const spans = traceCollector.getSpansForTrace(traceId)
           if (spans) {
             req.serverTimings = spansToServerTimings(spans, true)
           }
-          traceCollector.clearTrace(traceId)
         }
       } catch (e) {
         logSchema.error(logger, 'failed parsing server times on abort', { error: e, type: 'otel' })
+      } finally {
+        if (traceId) {
+          traceCollector.clearTrace(traceId)
+        }
       }
     })
   },

diff --git a/src/internal/monitoring/otel.ts b/src/internal/monitoring/otel.ts
@@ -16,7 +16,7 @@ import { ATTR_SERVICE_NAME, ATTR_SERVICE_VERSION } from '@opentelemetry/semantic
 import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc'
 import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node'
 import { CompressionAlgorithm } from '@opentelemetry/otlp-exporter-base'
-import { SpanExporter, BatchSpanProcessor } from '@opentelemetry/sdk-trace-base'
+import { SpanExporter, BatchSpanProcessor, SpanProcessor } from '@opentelemetry/sdk-trace-base'
 import * as grpc from '@grpc/grpc-js'
 import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'
 import { IncomingMessage } from 'node:http'
@@ -36,6 +36,7 @@ import { StreamSplitter } from '@tus/server'
 
 const tracingEnabled = process.env.TRACING_ENABLED === 'true'
 const headersEnv = process.env.OTEL_EXPORTER_OTLP_TRACES_HEADERS || ''
+const enableLogTraces = ['debug', 'logs'].includes(process.env.TRACING_MODE || '')
 
 const exporterHeaders = headersEnv
   .split(',')
@@ -67,13 +68,23 @@ if (tracingEnabled && endpoint) {
 // Create a BatchSpanProcessor using the trace exporter
 const batchProcessor = traceExporter ? new BatchSpanProcessor(traceExporter) : undefined
 
+const spanProcessors: SpanProcessor[] = []
+
+if (batchProcessor) {
+  spanProcessors.push(batchProcessor)
+}
+
+if (enableLogTraces) {
+  spanProcessors.push(traceCollector)
+}
+
 // Configure the OpenTelemetry Node SDK
 const sdk = new NodeSDK({
   resource: new Resource({
     [ATTR_SERVICE_NAME]: 'storage',
     [ATTR_SERVICE_VERSION]: version,
   }),
-  spanProcessors: batchProcessor ? [traceCollector, batchProcessor] : [traceCollector],
+  spanProcessors: spanProcessors,
   traceExporter,
   instrumentations: [
     new HttpInstrumentation({
@@ -266,10 +277,8 @@ const sdk = new NodeSDK({
       enabled: true,
       methodsToInstrument: [
         'done',
-        '__doConcurrentUpload',
         '__uploadUsingPut',
         '__createMultipartUpload',
-        '__notifyProgress',
         'markUploadAsAborted',
       ],
     }),
@@ -304,7 +313,7 @@ const sdk = new NodeSDK({
   ],
 })
 
-if (tracingEnabled) {
+if (tracingEnabled && spanProcessors.length > 0) {
   // Initialize the OpenTelemetry Node SDK
   sdk.start()