From 896fe49f1fd273824bae9ee406544cc6fe22821c Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Sat, 17 Aug 2024 00:17:10 +0100
Subject: [PATCH] fix(relay): set better OTEL metadata (#6322)

Previously, the `service.name` attribute got overridden with "unknown
service" from the detector used in `Resource::default`. To avoid this,
we are now manually composing the two other detectors.

This gives us a useful set of default labels from within the code yet it
allows overriding all of them using `OTEL_RESOURCE_ATTRIBUTES`.
---
 rust/docker-init-relay.sh |  7 ++++---
 rust/relay/src/main.rs    | 34 ++++++++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/rust/docker-init-relay.sh b/rust/docker-init-relay.sh
index 47a19838c..47e57d0e2 100755
--- a/rust/docker-init-relay.sh
+++ b/rust/docker-init-relay.sh
@@ -24,13 +24,14 @@ fi
 if [ "${OTEL_METADATA_DISCOVERY_METHOD}" = "gce_metadata" ]; then
     echo "Using GCE metadata to set OTEL metadata"
 
-    instance_id=$(curl "http://metadata.google.internal/computeMetadata/v1/instance/id" -H "Metadata-Flavor: Google" -s)
-    instance_name=$(curl http://metadata.google.internal/computeMetadata/v1/instance/name -H "Metadata-Flavor: Google" -s)
+    instance_id=$(curl "http://metadata.google.internal/computeMetadata/v1/instance/id" -H "Metadata-Flavor: Google" -s)           # i.e. 5832583187537235075
+    instance_name=$(curl "http://metadata.google.internal/computeMetadata/v1/instance/name" -H "Metadata-Flavor: Google" -s)       # i.e. relay-m5k7
+    zone=$(curl "http://metadata.google.internal/computeMetadata/v1/instance/zone" -H "Metadata-Flavor: Google" -s | cut -d/ -f4)  # i.e. us-east-1
 
     # Source for attribute names:
     # - https://opentelemetry.io/docs/specs/semconv/attributes-registry/service/
     # - https://opentelemetry.io/docs/specs/semconv/attributes-registry/gcp/#gcp---google-compute-engine-gce-attributes:
-    export OTEL_RESOURCE_ATTRIBUTES="service.instance.id=${instance_id},gcp.gce.instance.name=${instance_name}"
+    export OTEL_RESOURCE_ATTRIBUTES="service.instance.id=${instance_id},gcp.gce.instance.name=${instance_name},cloud.region=${zone}"
     echo "Discovered OTEL metadata: ${OTEL_RESOURCE_ATTRIBUTES}"
 fi
 
diff --git a/rust/relay/src/main.rs b/rust/relay/src/main.rs
index 9b8860ef4..148dd569c 100644
--- a/rust/relay/src/main.rs
+++ b/rust/relay/src/main.rs
@@ -178,9 +178,9 @@ async fn main() -> Result<()> {
 ///
 /// If the user has specified [`TraceCollector::Otlp`], we will set up an OTLP-exporter that connects to an OTLP collector specified at `Args.otlp_grpc_endpoint`.
 fn setup_tracing(args: &Args) -> Result<()> {
-    use opentelemetry::{global, trace::TracerProvider as _, KeyValue};
+    use opentelemetry::{global, trace::TracerProvider as _};
     use opentelemetry_otlp::WithExportConfig;
-    use opentelemetry_sdk::{runtime::Tokio, trace::Config, Resource};
+    use opentelemetry_sdk::{runtime::Tokio, trace::Config};
 
     // Use `tracing_core` directly for the temp logger because that one does not initialize a `log` logger.
     // A `log` Logger cannot be unset once set, so we can't use that for our temp logger during the setup.
@@ -194,12 +194,7 @@ fn setup_tracing(args: &Args) -> Result<()> {
             .with(env_filter())
             .into(),
         Some(endpoint) => {
-            let default_metadata = Resource::new([
-                KeyValue::new("service.name", "relay"),
-                KeyValue::new("service.namespace", "firezone"),
-            ]);
-            let metadata = default_metadata.merge(&Resource::default()); // `Resource::default` fetches from env-variables.
-
+            let metadata = make_otel_metadata();
             let grpc_endpoint = format!("http://{endpoint}");
 
             tracing::trace!(target: "relay", %grpc_endpoint, "Setting up OTLP exporter for collector");
@@ -619,6 +614,29 @@ fn is_healthy(last_heartbeat_sent: Arc<Mutex<Option<Instant>>>) -> bool {
     last_hearbeat_sent.elapsed() < MAX_PARTITION_TIME
 }
 
+fn make_otel_metadata() -> opentelemetry_sdk::Resource {
+    use opentelemetry::{Key, KeyValue};
+    use opentelemetry_sdk::resource::{EnvResourceDetector, TelemetryResourceDetector};
+    use opentelemetry_sdk::Resource;
+
+    const SERVICE_NAME: Key = Key::from_static_str("service.name");
+    const SERVICE_NAMESPACE: Key = Key::from_static_str("service.namespace");
+
+    let default_metadata = Resource::new([
+        KeyValue::new(SERVICE_NAMESPACE, "firezone"),
+        KeyValue::new(SERVICE_NAME, "relay"),
+    ]);
+    let detected_metadata = Resource::from_detectors(
+        Duration::ZERO,
+        vec![
+            Box::new(TelemetryResourceDetector),
+            Box::new(EnvResourceDetector::new()), // Allow overriding metadata using `OTEL_RESOURCE_ATTRIBUTES` env var.
+        ],
+    );
+
+    default_metadata.merge(&detected_metadata)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;