From 4c07606da66cb60239e49f799ebc8a4e008bbd96 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 21 Feb 2024 17:51:10 +0800 Subject: [PATCH] refactor: put together HTTP headers (#3337) * refactor: put together HTTP headers Signed-off-by: tison * do refactor Signed-off-by: tison * drop dirty commit Signed-off-by: tison * reduce changeset Signed-off-by: tison * fixup compilations Signed-off-by: tison * tidy files Signed-off-by: tison * drop common-api Signed-off-by: tison * fmt Signed-off-by: tison --------- Signed-off-by: tison --- src/common/error/src/lib.rs | 6 ++- src/servers/src/error.rs | 4 +- src/servers/src/http/arrow_result.rs | 8 ++-- src/servers/src/http/csv_result.rs | 4 +- src/servers/src/http/error_result.rs | 6 +-- src/servers/src/http/greptime_result_v1.rs | 6 +-- src/servers/src/http/header.rs | 43 ++++++++++++++++++---- src/servers/src/http/influxdb_result_v1.rs | 4 +- src/servers/src/http/prometheus_resp.rs | 2 +- src/servers/tests/http/influxdb_test.rs | 4 +- 10 files changed, 60 insertions(+), 27 deletions(-) diff --git a/src/common/error/src/lib.rs b/src/common/error/src/lib.rs index aa3c915e84e3..aa54ef39e78f 100644 --- 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 index e4b25adcb305..4ebbdc55445c 100644 --- 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::::with_capacity(2); diff --git a/src/servers/src/http/arrow_result.rs b/src/servers/src/http/arrow_result.rs index b47912b1617e..3daad34f1d00 100644 --- 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 index 30c5d4a0264d..f0d377b01e54 100644 --- 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 index 629594e66456..57a4bd698105 100644 --- 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 index 596f1bcfdd8f..1efeeccda25c 100644 --- 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 index aa0970dbdb3c..fd5dc8c43038 100644 --- 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); diff --git a/src/servers/src/http/influxdb_result_v1.rs b/src/servers/src/http/influxdb_result_v1.rs index a4e8206058df..05525ea128a6 100644 --- 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 index 3deb0b109143..e7a310faf5b0 100644 --- 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 index 81f17181093b..9b68802bb4c6 100644 --- 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!(