fix pr comments

Signed-off-by: Adar Ovadia <[email protected]>
valkey-io · Feb 3, 2025 · 28edfda · 28edfda
1 parent 9edfc86
commit 28edfda
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 14 deletions.
diff --git a/glide-core/redis-rs/redis/src/cluster_client.rs b/glide-core/redis-rs/redis/src/cluster_client.rs
@@ -408,9 +408,6 @@ impl ClusterClientBuilder {
     ///
     /// # Parameters
     /// - `open_telemetry_config`: Use the `open_telemetry_config` property to specify the endpoint of the collector to export the measurments.
-    ///  - **Collector EndPoint**: Set `collectorEndPoint` to specify the endpoint of the collector to export the measurments.
-    ///  - **Span Flush Interval**: Set `spanFlushInterval` to specify the duration in milliseconds the data will be exported to the collector. If interval is not specified, 5000 will be used.
-    ///
     pub fn open_telemetry_config(
         mut self,
         open_telemetry_config: GlideOpenTelemetryConfig,

diff --git a/glide-core/src/client/mod.rs b/glide-core/src/client/mod.rs
@@ -806,12 +806,14 @@ impl Client {
             inflight_requests_limit.try_into().unwrap(),
         ));
 
-        if let Some(endpoint_str) = &request.open_telemetry_endpoint {
+        if let Some(endpoint_str) = &request.otel_endpoint {
             let trace_exporter = GlideOpenTelemetryTraceExporter::from_str(endpoint_str.as_str())
                 .map_err(ConnectionError::IoError)?;
             let config = GlideOpenTelemetryConfigBuilder::default()
                 .with_flush_interval(std::time::Duration::from_millis(
-                    request.open_telemetry_span_interval.unwrap_or(5000),
+                    request
+                        .otel_span_flush_interval_ms
+                        .unwrap_or(DEFAULT_FLUSH_SPAN_INTERVAL_MS),
                 ))
                 .with_trace_exporter(trace_exporter)
                 .build();

diff --git a/glide-core/src/client/types.rs b/glide-core/src/client/types.rs
@@ -25,8 +25,8 @@ pub struct ConnectionRequest {
     pub periodic_checks: Option<PeriodicCheck>,
     pub pubsub_subscriptions: Option<redis::PubSubSubscriptionInfo>,
     pub inflight_requests_limit: Option<u32>,
-    pub open_telemetry_endpoint: Option<String>,
-    pub open_telemetry_span_interval: Option<u64>,
+    pub otel_endpoint: Option<String>,
+    pub otel_span_flush_interval_ms: Option<u64>,
 }
 
 pub struct AuthenticationInfo {
@@ -208,9 +208,8 @@ impl From<protobuf::ConnectionRequest> for ConnectionRequest {
 
         let inflight_requests_limit = none_if_zero(value.inflight_requests_limit);
 
-        let open_telemetry_endpoint =
-            chars_to_string_option(&value.opentelemetry_config.collector_end_point);
-        let open_telemetry_span_interval = value.opentelemetry_config.span_flush_interval;
+        let otel_endpoint = chars_to_string_option(&value.opentelemetry_config.collector_end_point);
+        let otel_span_flush_interval_ms = value.opentelemetry_config.span_flush_interval;
 
         ConnectionRequest {
             read_from,
@@ -227,8 +226,8 @@ impl From<protobuf::ConnectionRequest> for ConnectionRequest {
             periodic_checks,
             pubsub_subscriptions,
             inflight_requests_limit,
-            open_telemetry_endpoint,
-            open_telemetry_span_interval,
+            otel_endpoint,
+            otel_span_flush_interval_ms,
         }
     }
 }
diff --git a/glide-core/telemetry/src/open_telemetry.rs b/glide-core/telemetry/src/open_telemetry.rs
@@ -13,6 +13,9 @@ const SPAN_WRITE_LOCK_ERR: &str = "Failed to get span write lock";
 const SPAN_READ_LOCK_ERR: &str = "Failed to get span read lock";
 const TRACE_SCOPE: &str = "valkey_glide";
 
+/// Default interval in milliseconds for flushing open telemetry data to the collector.
+pub const DEFAULT_FLUSH_SPAN_INTERVAL_MS: u64 = 5000;
+
 pub enum GlideSpanStatus {
     Ok,
     Error(String),
@@ -46,11 +49,16 @@ fn parse_endpoint(endpoint: &str) -> Result<GlideOpenTelemetryTraceExporter, Err
         .map_err(|_| Error::new(ErrorKind::InvalidInput, format!("Parse error. {endpoint}")))?;
 
     match url.scheme() {
-        "http" | "https" => Ok(GlideOpenTelemetryTraceExporter::Http(format!(
+        "http" => Ok(GlideOpenTelemetryTraceExporter::Http(format!(
             "{}:{}",
             url.host_str().unwrap_or("127.0.0.1"),
             url.port().unwrap_or(80)
         ))), // HTTP endpoint
+        "https" => Ok(GlideOpenTelemetryTraceExporter::Http(format!(
+            "{}:{}",
+            url.host_str().unwrap_or("127.0.0.1"),
+            url.port().unwrap_or(443)
+        ))), // HTTPS endpoint
         "grpc" => Ok(GlideOpenTelemetryTraceExporter::Grpc(format!(
             "{}:{}",
             url.host_str().unwrap_or("127.0.0.1"),
@@ -235,7 +243,7 @@ pub struct GlideOpenTelemetryConfigBuilder {
 impl Default for GlideOpenTelemetryConfigBuilder {
     fn default() -> Self {
         GlideOpenTelemetryConfigBuilder {
-            span_flush_interval: std::time::Duration::from_millis(5_000),
+            span_flush_interval: std::time::Duration::from_millis(DEFAULT_FLUSH_SPAN_INTERVAL_MS),
             trace_exporter: GlideOpenTelemetryTraceExporter::File(std::env::temp_dir()),
         }
     }