Merge pull request #9258 from OpenMined/yash/otel-config

Configure OTLP endpoint through Helm
OpenMined · Sep 4, 2024 · 1096dd2 · 1096dd2
2 parents 91e98f7 + e7bbcc7
commit 1096dd2
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 15 deletions.
diff --git a/packages/grid/backend/grid/start.sh b/packages/grid/backend/grid/start.sh
@@ -12,8 +12,6 @@ APPDIR=${APPDIR:-$HOME/app}
 RELOAD=""
 ROOT_PROC=""
 
-echo "Starting with TRACING=${TRACING}"
-
 if [[ ${DEV_MODE} == "True" ]];
 then
     echo "Hot-reload Enabled"
@@ -30,18 +28,10 @@ fi
 
 if [[ ${TRACING} == "true" ]];
 then
-    echo "OpenTelemetry Enabled"
-
     # TODOs:
-    # ! Handle case when OTEL_EXPORTER_OTLP_ENDPOINT is not set.
-    # ! syft-signoz-otel-collector.platform:4317 should be plumbed through helm charts
     # ? Kubernetes OTel operator is recommended by signoz
     export OTEL_PYTHON_LOG_CORRELATION=${OTEL_PYTHON_LOG_CORRELATION:-true}
-    export OTEL_EXPORTER_OTLP_ENDPOINT=${OTEL_EXPORTER_OTLP_ENDPOINT:-"http://syft-signoz-otel-collector.platform:4317"}
-    export OTEL_EXPORTER_OTLP_PROTOCOL=${OTEL_EXPORTER_OTLP_PROTOCOL:-grpc}
-
-    # TODO: uvicorn postfork is not stable with OpenTelemetry
-    # ROOT_PROC="opentelemetry-instrument"
+    echo "OpenTelemetry Enabled. Endpoint=$OTEL_EXPORTER_OTLP_ENDPOINT Protocol=$OTEL_EXPORTER_OTLP_PROTOCOL"
 else
     echo "OpenTelemetry Disabled"
 fi

diff --git a/packages/grid/devspace.yaml b/packages/grid/devspace.yaml
@@ -130,7 +130,10 @@ profiles:
       - op: add
         path: deployments.syft.helm.values.server
         value:
-          tracing: true
+          tracing:
+            enabled: true
+            otlpEndpoint: "http://syft-signoz-otel-collector.platform:4317"
+            otelProtocol: "grpc"
 
   - name: bigquery-scenario-tests
     description: "Deploy a datasite for bigquery scenario testing"

diff --git a/packages/grid/helm/examples/dev/base.yaml b/packages/grid/helm/examples/dev/base.yaml
@@ -7,7 +7,8 @@ global:
 server:
   rootEmail: [email protected]
   associationRequestAutoApproval: true
-  tracing: false
+  tracing:
+    enabled: false
 
   resourcesPreset: null
   resources: null

diff --git a/packages/grid/helm/syft/templates/backend/backend-statefulset.yaml b/packages/grid/helm/syft/templates/backend/backend-statefulset.yaml
@@ -141,9 +141,15 @@ spec:
             {{- end }}
             - name: MIN_SIZE_BLOB_STORAGE_MB
               value: {{ .Values.seaweedfs.minSizeBlobStorageMB | quote }}
+            {{- if .Values.server.tracing.enabled }}
             # Tracing
             - name: TRACING
-              value: {{ .Values.server.tracing | default "False" | quote }}
+              value: {{ .Values.server.tracing.enabled | default "False" | quote }}
+            - name: OTEL_EXPORTER_OTLP_ENDPOINT
+              value: {{ .Values.server.tracing.otlpEndpoint | required "server.tracing.otlpEndpoint is required" | quote }}
+            - name: OTEL_EXPORTER_OTLP_PROTOCOL
+              value: {{ .Values.server.tracing.otlpProtocol | default "grpc" | quote }}
+            {{- end }}
             # Enclave attestation
             {{- if .Values.attestation.enabled }}
             - name: ENCLAVE_ATTESTATION_ENABLED

diff --git a/packages/grid/helm/syft/values.yaml b/packages/grid/helm/syft/values.yaml
@@ -173,7 +173,11 @@ server:
   debuggerEnabled: false
   associationRequestAutoApproval: false
   useInternalRegistry: true
-  tracing: false
+  tracing:
+    enabled: false
+    # OpenTelemetry Collector endpoint
+    otlpEndpoint: ""
+    otlpProtocol: grpc
 
   # Default Worker pool settings
   defaultWorkerPool: