refactor: put together HTTP headers (GreptimeTeam#3337)

* refactor: put together HTTP headers Signed-off-by: tison <[email protected]> * do refactor Signed-off-by: tison <[email protected]> * drop dirty commit Signed-off-by: tison <[email protected]> * reduce changeset Signed-off-by: tison <[email protected]> * fixup compilations Signed-off-by: tison <[email protected]> * tidy files Signed-off-by: tison <[email protected]> * drop common-api Signed-off-by: tison <[email protected]> * fmt Signed-off-by: tison <[email protected]> --------- Signed-off-by: tison <[email protected]>
evenyag · Feb 21, 2024 · 4c07606 · 4c07606
1 parent a7bf458
commit 4c07606
Show file tree

Hide file tree

Showing 10 changed files with 60 additions and 27 deletions.
diff --git a/src/common/error/src/lib.rs b/src/common/error/src/lib.rs
@@ -19,7 +19,9 @@ pub mod format;
 pub mod mock;
 pub mod status_code;
 
+pub use snafu;
+
+// HACK - these headers are here for shared in gRPC services. For common HTTP headers,
+// please define in `src/servers/src/http/header.rs`.
 pub const GREPTIME_DB_HEADER_ERROR_CODE: &str = "x-greptime-err-code";
 pub const GREPTIME_DB_HEADER_ERROR_MSG: &str = "x-greptime-err-msg";
-
-pub use snafu;
diff --git a/src/servers/src/error.rs b/src/servers/src/error.rs
@@ -596,9 +596,11 @@ macro_rules! define_into_tonic_status {
     ($Error: ty) => {
         impl From<$Error> for tonic::Status {
             fn from(err: $Error) -> Self {
-                use common_error::{GREPTIME_DB_HEADER_ERROR_CODE, GREPTIME_DB_HEADER_ERROR_MSG};
                 use tonic::codegen::http::{HeaderMap, HeaderValue};
                 use tonic::metadata::MetadataMap;
+                use $crate::http::header::constants::{
+                    GREPTIME_DB_HEADER_ERROR_CODE, GREPTIME_DB_HEADER_ERROR_MSG,
+                };
 
                 let mut headers = HeaderMap::<HeaderValue>::with_capacity(2);
 

diff --git a/src/servers/src/http/arrow_result.rs b/src/servers/src/http/arrow_result.rs
@@ -17,7 +17,7 @@ use std::sync::Arc;
 
 use arrow::datatypes::Schema;
 use arrow_ipc::writer::FileWriter;
-use axum::http::{header, HeaderName, HeaderValue};
+use axum::http::{header, HeaderValue};
 use axum::response::{IntoResponse, Response};
 use common_error::status_code::StatusCode;
 use common_query::Output;
@@ -122,15 +122,15 @@ impl IntoResponse for ArrowResponse {
         (
             [
                 (
-                    header::CONTENT_TYPE,
+                    &header::CONTENT_TYPE,
                     HeaderValue::from_static("application/arrow"),
                 ),
                 (
-                    HeaderName::from_static(GREPTIME_DB_HEADER_FORMAT),
+                    &GREPTIME_DB_HEADER_FORMAT,
                     HeaderValue::from_static("ARROW"),
                 ),
                 (
-                    HeaderName::from_static(GREPTIME_DB_HEADER_EXECUTION_TIME),
+                    &GREPTIME_DB_HEADER_EXECUTION_TIME,
                     HeaderValue::from(execution_time),
                 ),
             ],

diff --git a/src/servers/src/http/csv_result.rs b/src/servers/src/http/csv_result.rs
@@ -101,9 +101,9 @@ impl IntoResponse for CsvResponse {
         )
             .into_response();
         resp.headers_mut()
-            .insert(GREPTIME_DB_HEADER_FORMAT, HeaderValue::from_static("CSV"));
+            .insert(&GREPTIME_DB_HEADER_FORMAT, HeaderValue::from_static("CSV"));
         resp.headers_mut().insert(
-            GREPTIME_DB_HEADER_EXECUTION_TIME,
+            &GREPTIME_DB_HEADER_EXECUTION_TIME,
             HeaderValue::from(execution_time),
         );
         resp

diff --git a/src/servers/src/http/error_result.rs b/src/servers/src/http/error_result.rs
@@ -17,11 +17,11 @@ use axum::response::{IntoResponse, Response};
 use axum::Json;
 use common_error::ext::ErrorExt;
 use common_error::status_code::StatusCode;
-use common_error::{GREPTIME_DB_HEADER_ERROR_CODE, GREPTIME_DB_HEADER_ERROR_MSG};
 use common_telemetry::logging::{debug, error};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
+use crate::http::header::constants::{GREPTIME_DB_HEADER_ERROR_CODE, GREPTIME_DB_HEADER_ERROR_MSG};
 use crate::http::header::{GREPTIME_DB_HEADER_EXECUTION_TIME, GREPTIME_DB_HEADER_FORMAT};
 use crate::http::ResponseFormat;
 
@@ -88,9 +88,9 @@ impl IntoResponse for ErrorResponse {
             HeaderValue::from_str(&msg).expect("malformed error msg"),
         );
         resp.headers_mut()
-            .insert(GREPTIME_DB_HEADER_FORMAT, HeaderValue::from_static(ty));
+            .insert(&GREPTIME_DB_HEADER_FORMAT, HeaderValue::from_static(ty));
         resp.headers_mut().insert(
-            GREPTIME_DB_HEADER_EXECUTION_TIME,
+            &GREPTIME_DB_HEADER_EXECUTION_TIME,
             HeaderValue::from(execution_time),
         );
         resp

diff --git a/src/servers/src/http/greptime_result_v1.rs b/src/servers/src/http/greptime_result_v1.rs
@@ -76,15 +76,15 @@ impl IntoResponse for GreptimedbV1Response {
         let mut resp = Json(self).into_response();
 
         resp.headers_mut().insert(
-            GREPTIME_DB_HEADER_FORMAT,
+            &GREPTIME_DB_HEADER_FORMAT,
             HeaderValue::from_static("greptimedb_v1"),
         );
         resp.headers_mut().insert(
-            GREPTIME_DB_HEADER_EXECUTION_TIME,
+            &GREPTIME_DB_HEADER_EXECUTION_TIME,
             HeaderValue::from(execution_time),
         );
         if let Some(m) = metrics.and_then(|m| HeaderValue::from_str(&m).ok()) {
-            resp.headers_mut().insert(GREPTIME_DB_HEADER_METRICS, m);
+            resp.headers_mut().insert(&GREPTIME_DB_HEADER_METRICS, m);
         }
 
         resp

diff --git a/src/servers/src/http/header.rs b/src/servers/src/http/header.rs
@@ -14,16 +14,45 @@
 
 use headers::{Header, HeaderName, HeaderValue};
 
-pub const GREPTIME_DB_HEADER_FORMAT: &str = "x-greptime-format";
-pub const GREPTIME_DB_HEADER_EXECUTION_TIME: &str = "x-greptime-execution-time";
-pub const GREPTIME_DB_HEADER_METRICS: &str = "x-greptime-metrics";
+pub mod constants {
+    // New HTTP headers would better distinguish use cases among:
+    // * GreptimeDB
+    // * GreptimeCloud
+    // * ...
+    //
+    // And thus trying to use:
+    // * x-greptime-db-xxx
+    // * x-greptime-cloud-xxx
+    //
+    // ... accordingly
+    //
+    // Most of the headers are for GreptimeDB and thus using `x-greptime-db-` as prefix.
+    // Only use `x-greptime-cloud` when it's intentionally used by GreptimeCloud.
+
+    // LEGACY HEADERS - KEEP IT UNMODIFIED
+    pub const GREPTIME_DB_HEADER_FORMAT: &str = "x-greptime-format";
+    pub const GREPTIME_DB_HEADER_EXECUTION_TIME: &str = "x-greptime-execution-time";
+    pub const GREPTIME_DB_HEADER_METRICS: &str = "x-greptime-metrics";
+    pub const GREPTIME_DB_HEADER_NAME: &str = "x-greptime-db-name";
+    pub const GREPTIME_TIMEZONE_HEADER_NAME: &str = "x-greptime-timezone";
+    pub const GREPTIME_DB_HEADER_ERROR_CODE: &str = common_error::GREPTIME_DB_HEADER_ERROR_CODE;
+    pub const GREPTIME_DB_HEADER_ERROR_MSG: &str = common_error::GREPTIME_DB_HEADER_ERROR_MSG;
+}
+
+pub static GREPTIME_DB_HEADER_FORMAT: HeaderName =
+    HeaderName::from_static(constants::GREPTIME_DB_HEADER_FORMAT);
+pub static GREPTIME_DB_HEADER_EXECUTION_TIME: HeaderName =
+    HeaderName::from_static(constants::GREPTIME_DB_HEADER_EXECUTION_TIME);
+pub static GREPTIME_DB_HEADER_METRICS: HeaderName =
+    HeaderName::from_static(constants::GREPTIME_DB_HEADER_METRICS);
 
 /// Header key of `db-name`. Example format of the header value is `greptime-public`.
-pub static GREPTIME_DB_HEADER_NAME: HeaderName = HeaderName::from_static("x-greptime-db-name");
-/// Header key of query specific timezone.
-/// Example format of the header value is `Asia/Shanghai` or `+08:00`.
+pub static GREPTIME_DB_HEADER_NAME: HeaderName =
+    HeaderName::from_static(constants::GREPTIME_DB_HEADER_NAME);
+
+/// Header key of query specific timezone. Example format of the header value is `Asia/Shanghai` or `+08:00`.
 pub static GREPTIME_TIMEZONE_HEADER_NAME: HeaderName =
-    HeaderName::from_static("x-greptime-timezone");
+    HeaderName::from_static(constants::GREPTIME_TIMEZONE_HEADER_NAME);
 
 pub struct GreptimeDbName(Option<String>);
 

diff --git a/src/servers/src/http/influxdb_result_v1.rs b/src/servers/src/http/influxdb_result_v1.rs
@@ -217,11 +217,11 @@ impl IntoResponse for InfluxdbV1Response {
         let execution_time = self.execution_time_ms;
         let mut resp = Json(self).into_response();
         resp.headers_mut().insert(
-            GREPTIME_DB_HEADER_FORMAT,
+            &GREPTIME_DB_HEADER_FORMAT,
             HeaderValue::from_static("influxdb_v1"),
         );
         resp.headers_mut().insert(
-            GREPTIME_DB_HEADER_EXECUTION_TIME,
+            &GREPTIME_DB_HEADER_EXECUTION_TIME,
             HeaderValue::from(execution_time),
         );
         resp

diff --git a/src/servers/src/http/prometheus_resp.rs b/src/servers/src/http/prometheus_resp.rs
@@ -66,7 +66,7 @@ impl IntoResponse for PrometheusJsonResponse {
         let mut resp = Json(self).into_response();
 
         if let Some(m) = metrics.and_then(|m| HeaderValue::from_str(&m).ok()) {
-            resp.headers_mut().insert(GREPTIME_DB_HEADER_METRICS, m);
+            resp.headers_mut().insert(&GREPTIME_DB_HEADER_METRICS, m);
         }
 
         resp

diff --git a/src/servers/tests/http/influxdb_test.rs b/src/servers/tests/http/influxdb_test.rs
@@ -169,7 +169,7 @@ async fn test_influxdb_write() {
         .await;
     assert_eq!(result.status(), 401);
     assert_eq!(
-        result.headers().get(GREPTIME_DB_HEADER_FORMAT).unwrap(),
+        result.headers().get(&GREPTIME_DB_HEADER_FORMAT).unwrap(),
         "influxdb_v1",
     );
     assert_eq!(
@@ -185,7 +185,7 @@ async fn test_influxdb_write() {
         .await;
     assert_eq!(result.status(), 401);
     assert_eq!(
-        result.headers().get(GREPTIME_DB_HEADER_FORMAT).unwrap(),
+        result.headers().get(&GREPTIME_DB_HEADER_FORMAT).unwrap(),
         "influxdb_v1",
     );
     assert_eq!(