Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support http/json protocol #1585

Merged
merged 40 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
1482d5c
Add http-json feature flag
ramgdev Feb 27, 2024
576a56d
prototype json serialization
ramgdev Feb 27, 2024
d13ab4b
fix default protocol
ramgdev Feb 27, 2024
ed4fc03
fix typo
ramgdev Feb 27, 2024
587c341
use default protocol for HttpConfig
ramgdev Feb 28, 2024
d460bda
Update opentelemetry-otlp/src/exporter/http/trace.rs
ramgdev Feb 28, 2024
da76734
address review comments
ramgdev Feb 29, 2024
aa74b9c
run pre-commit script
ramgdev Feb 29, 2024
efa8588
fix dependency
ramgdev Feb 29, 2024
e47a80f
Merge branch 'main' into otel-json
ramgdev Mar 3, 2024
04593ec
Update mod.rs
ramgdev Mar 3, 2024
277df67
unit test default endpoint
ramgdev Mar 3, 2024
5d5e06e
test default protocol
ramgdev Mar 4, 2024
359babe
Merge branch 'main' into otel-json
ramgdev Mar 12, 2024
8c9577f
Merge branch 'main' into otel-json
ramgdev Mar 12, 2024
f175980
Merge branch 'main' into otel-json
ramgdev Mar 15, 2024
f93058c
enable http-json for all signals
ramgdev Mar 18, 2024
46f92c4
Merge branch 'main' into otel-json
ramgdev Mar 18, 2024
0f1a805
fix lint
ramgdev Mar 18, 2024
6ebea63
Merge branch 'main' into otel-json
ramgdev Mar 19, 2024
e50ac0e
run precommit
ramgdev Mar 19, 2024
049e96a
Merge branch 'main' into otel-json
ramgdev Mar 20, 2024
d64bda0
Merge branch 'main' into otel-json
ramgdev Mar 24, 2024
ad8322b
add unit test
ramgdev Mar 25, 2024
e457a85
Merge branch 'main' into otel-json
ramgdev Mar 25, 2024
5222f8d
Merge branch 'main' into otel-json
ramgdev Mar 25, 2024
25b0857
Merge branch 'main' into otel-json
ramgdev Mar 29, 2024
63fb044
update changelog
ramgdev Mar 29, 2024
bcde320
Merge branch 'main' into otel-json
ramgdev Mar 29, 2024
f9abf24
address code review
ramgdev Mar 29, 2024
250fab4
remove duplicate line
ramgdev Mar 29, 2024
67552f8
Update opentelemetry-otlp/src/exporter/http/metrics.rs
ramgdev Apr 1, 2024
765ab99
Merge branch 'main' into otel-json
ramgdev Apr 5, 2024
67d6875
Merge branch 'main' into otel-json
ramgdev Apr 15, 2024
680b191
Merge branch 'main' into otel-json
lalitb Apr 16, 2024
c6d1817
Merge branch 'main' into otel-json
ramgdev Apr 20, 2024
4ed5158
move build_body to otlphttpclient
ramgdev Apr 20, 2024
ebc3cc3
allow builder to accept protocol
ramgdev Apr 20, 2024
86a89d5
Merge branch 'main' into otel-json
ramgdev Apr 21, 2024
aa2cd05
Merge branch 'main' into otel-json
TommyCpp Apr 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion opentelemetry-otlp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ reqwest = { workspace = true, optional = true }
http = { workspace = true, optional = true }
serde = { workspace = true, features = ["derive"], optional = true }
thiserror = { workspace = true }
serde_json = { workspace = true, optional = true }

[dev-dependencies]
tokio-stream = { workspace = true, features = ["net"] }
Expand All @@ -58,7 +59,7 @@ metrics = ["opentelemetry/metrics", "opentelemetry_sdk/metrics", "opentelemetry-
logs = ["opentelemetry/logs", "opentelemetry_sdk/logs", "opentelemetry-proto/logs"]

# add ons
serialize = ["serde"]
serialize = ["serde", "serde_json"]

default = ["grpc-tonic", "trace"]

Expand All @@ -73,6 +74,8 @@ http-proto = ["prost", "opentelemetry-http", "opentelemetry-proto/gen-tonic-mess
reqwest-blocking-client = ["reqwest/blocking", "opentelemetry-http/reqwest"]
reqwest-client = ["reqwest", "opentelemetry-http/reqwest"]
reqwest-rustls = ["reqwest", "reqwest/rustls-tls-native-roots"]
# http json
http-json = ["serde_json", "prost", "opentelemetry-http", "opentelemetry-proto/gen-tonic-messages", "opentelemetry-proto/with-serde", "http", "trace", "metrics"]
ramgdev marked this conversation as resolved.
Show resolved Hide resolved

# test
integration-testing = ["tonic", "prost", "tokio/full", "trace"]
2 changes: 1 addition & 1 deletion opentelemetry-otlp/examples/basic-otlp-http/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ publish = false
once_cell = { workspace = true }
opentelemetry = { path = "../../../opentelemetry" }
opentelemetry_sdk = { path = "../../../opentelemetry-sdk", features = ["rt-tokio", "metrics", "logs"] }
opentelemetry-otlp = { path = "../..", features = ["http-proto", "reqwest-client", "logs"] }
opentelemetry-otlp = { path = "../..", features = ["http-proto", "http-json", "reqwest-client", "logs"] }
opentelemetry-appender-tracing = { path = "../../../opentelemetry-appender-tracing", default-features = false}
opentelemetry-semantic-conventions = { path = "../../../opentelemetry-semantic-conventions" }

Expand Down
24 changes: 19 additions & 5 deletions opentelemetry-otlp/src/exporter/http/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,32 @@ impl LogExporter for OtlpHttpClient {
}
}

#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
fn build_body(logs: Vec<LogData>) -> LogResult<(Vec<u8>, &'static str)> {
use crate::exporter::default_protocol;
#[cfg(feature = "http-json")]
use crate::Protocol;
use opentelemetry_proto::tonic::collector::logs::v1::ExportLogsServiceRequest;
use prost::Message;

let req = ExportLogsServiceRequest {
resource_logs: logs.into_iter().map(Into::into).collect(),
};
let mut buf = vec![];
req.encode(&mut buf).map_err(crate::Error::from)?;

Ok((buf, "application/x-protobuf"))
let buf;
let ctype;
match default_protocol() {
ramgdev marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(feature = "http-json")]
Protocol::HttpJson => {
let json_struct = serde_json::to_string_pretty(&req).unwrap();
buf = json_struct.into();
ctype = "application/json";
}
_ => {
buf = req.encode_to_vec();
ctype = "application/x-protobuf";
}
};
Ok((buf, ctype))
}

#[cfg(not(feature = "http-proto"))]
Expand Down
25 changes: 20 additions & 5 deletions opentelemetry-otlp/src/exporter/http/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,35 @@ impl MetricsClient for OtlpHttpClient {
}
}

#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
fn build_body(metrics: &mut ResourceMetrics) -> Result<(Vec<u8>, &'static str)> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this part. I wonder if we can make the feature additive.

One way I see we can do it is:

  • Add protocol field in OtelHttpClient
  • Move build_body as part of the OtelHttpClient.

Then we just ask users to pass protocol in building pipline(it's already part of the export_config. And based on the feature the corresponding protocol may be available

Copy link
Contributor Author

@ramgdev ramgdev Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we still talking about exporting in one format per signal per pipeline? If so, it should be possible with the above code, no? For e.g I can enable both features http-json and http-proto and build 2 trace pipelines that export in each of those formats. Let me know if I'm missing something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes by tuning the features it's possible to export in either proto or json. My preference is to ask users to explicitly tell the pipeline which protocol to rather then to config it using features. i.e it should be possible to enable both http-json and http-proto but choose to use http-proto. I think it current setup doesn't support this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. Would you want HttpExporterBuilder to receive protocol via with_protocol?

use crate::exporter::default_protocol;
#[cfg(feature = "http-json")]
use crate::Protocol;
use prost::Message;

let req: opentelemetry_proto::tonic::collector::metrics::v1::ExportMetricsServiceRequest =
(&*metrics).into();
let mut buf = vec![];
req.encode(&mut buf).map_err(crate::Error::from)?;

Ok((buf, "application/x-protobuf"))
let buf;
let ctype;
match default_protocol() {
#[cfg(feature = "http-json")]
Protocol::HttpJson => {
let json_struct = serde_json::to_string_pretty(&req).unwrap();
buf = json_struct.into();
ctype = "application/json";
}
_ => {
buf = req.encode_to_vec();
ctype = "application/x-protobuf";
}
};
Ok((buf, ctype))
}

#[cfg(not(feature = "http-proto"))]
fn build_body(metrics: &mut ResourceMetrics) -> Result<(Vec<u8>, &'static str)> {
fn build_body(_metrics: &mut ResourceMetrics) -> Result<(Vec<u8>, &'static str)> {
Err(MetricsError::Other(
"No http protocol configured. Enable one via `http-proto`".into(),
ramgdev marked this conversation as resolved.
Show resolved Hide resolved
))
Expand Down
8 changes: 4 additions & 4 deletions opentelemetry-otlp/src/exporter/http/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
ExportConfig, Protocol, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS,
ExportConfig, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS,
OTEL_EXPORTER_OTLP_TIMEOUT,
};
use http::{HeaderName, HeaderValue, Uri};
Expand All @@ -10,7 +10,7 @@ use std::str::FromStr;
use std::sync::{Arc, Mutex};
use std::time::Duration;

use super::{default_headers, parse_header_string};
use super::{default_headers, default_protocol, parse_header_string};

#[cfg(feature = "metrics")]
mod metrics;
Expand All @@ -22,7 +22,7 @@ mod logs;
mod trace;

/// Configuration of the http transport
#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
#[derive(Debug)]
#[cfg_attr(
all(
Expand Down Expand Up @@ -98,7 +98,7 @@ impl Default for HttpExporterBuilder {
fn default() -> Self {
HttpExporterBuilder {
exporter_config: ExportConfig {
protocol: Protocol::HttpBinary,
protocol: default_protocol(),
..ExportConfig::default()
},
http_config: HttpConfig {
Expand Down
28 changes: 21 additions & 7 deletions opentelemetry-otlp/src/exporter/http/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,37 @@ impl SpanExporter for OtlpHttpClient {
}
}

#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
fn build_body(spans: Vec<SpanData>) -> TraceResult<(Vec<u8>, &'static str)> {
use crate::exporter::default_protocol;
#[cfg(feature = "http-json")]
use crate::Protocol;
use opentelemetry_proto::tonic::collector::trace::v1::ExportTraceServiceRequest;
use prost::Message;

let req = ExportTraceServiceRequest {
resource_spans: spans.into_iter().map(Into::into).collect(),
};
let mut buf = vec![];
req.encode(&mut buf).map_err(crate::Error::from)?;

Ok((buf, "application/x-protobuf"))
let buf;
let ctype;
match default_protocol() {
#[cfg(feature = "http-json")]
Protocol::HttpJson => {
let json_struct = serde_json::to_string_pretty(&req).unwrap();
buf = json_struct.into();
ctype = "application/json";
}
_ => {
buf = req.encode_to_vec();
ctype = "application/x-protobuf";
}
};
Ok((buf, ctype))
}

#[cfg(not(feature = "http-proto"))]
#[cfg(not(any(feature = "http-proto", feature = "http-json")))]
fn build_body(spans: Vec<SpanData>) -> TraceResult<(Vec<u8>, &'static str)> {
Err(TraceError::Other(
"No http protocol configured. Enable one via `http-proto`".into(),
"No http protocol configured. Enable one via `http-proto` or `http-json`".into(),
))
}
90 changes: 69 additions & 21 deletions opentelemetry-otlp/src/exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! OTLP supports sending data via different protocols and formats.

#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
use crate::exporter::http::HttpExporterBuilder;
#[cfg(feature = "grpc-tonic")]
use crate::exporter::tonic::TonicExporterBuilder;
Expand All @@ -28,18 +28,32 @@
/// Compression algorithm to use, defaults to none.
pub const OTEL_EXPORTER_OTLP_COMPRESSION: &str = "OTEL_EXPORTER_OTLP_COMPRESSION";

#[cfg(feature = "http-proto")]
#[cfg(all(
feature = "http-json",
not(all(feature = "grpc-tonic", feature = "http-proto"))
))]
/// Default protocol, using http-json.
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON;
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(all(
feature = "http-proto",
not(all(feature = "grpc-tonic", feature = "http-json"))
))]
/// Default protocol, using http-proto.
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF;
#[cfg(all(feature = "grpc-tonic", not(feature = "http-proto")))]
/// Default protocol, using grpc as http-proto feature is not enabled.
#[cfg(all(
feature = "grpc-tonic",
not(all(feature = "http-proto", feature = "http-json"))
))]
/// Default protocol, using grpc
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_GRPC;
#[cfg(not(any(any(feature = "grpc-tonic", feature = "http-proto"))))]

#[cfg(not(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json")))]
/// Default protocol if no features are enabled.
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = "";
ramgdev marked this conversation as resolved.
Show resolved Hide resolved

const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF: &str = "http/protobuf";
const OTEL_EXPORTER_OTLP_PROTOCOL_GRPC: &str = "grpc";
const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON: &str = "http/json";

/// Max waiting time for the backend to process each signal batch, defaults to 10 seconds.
pub const OTEL_EXPORTER_OTLP_TIMEOUT: &str = "OTEL_EXPORTER_OTLP_TIMEOUT";
Expand All @@ -50,7 +64,7 @@
const OTEL_EXPORTER_OTLP_GRPC_ENDPOINT_DEFAULT: &str = "http://localhost:4317";
const OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT: &str = "http://localhost:4318";

#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
pub(crate) mod http;
#[cfg(feature = "grpc-tonic")]
pub(crate) mod tonic;
Expand Down Expand Up @@ -109,9 +123,10 @@

/// default protocol based on enabled features
fn default_protocol() -> Protocol {
match OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT {

Check failure on line 126 in opentelemetry-otlp/src/exporter/mod.rs

View workflow job for this annotation

GitHub Actions / lint

cannot find value `OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT` in this scope
OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF => Protocol::HttpBinary,
OTEL_EXPORTER_OTLP_PROTOCOL_GRPC => Protocol::Grpc,
OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON => Protocol::HttpJson,
_ => Protocol::HttpBinary,
}
}
Expand All @@ -121,11 +136,12 @@
match protocol {
Protocol::Grpc => OTEL_EXPORTER_OTLP_GRPC_ENDPOINT_DEFAULT.to_string(),
Protocol::HttpBinary => OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT.to_string(),
Protocol::HttpJson => OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT.to_string(),
}
}

/// default user-agent headers
#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
#[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
fn default_headers() -> std::collections::HashMap<String, String> {
let mut headers = std::collections::HashMap::new();
headers.insert(
Expand All @@ -148,7 +164,7 @@
}
}

#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
impl HasExportConfig for HttpExporterBuilder {
fn export_config(&mut self) -> &mut ExportConfig {
&mut self.exporter_config
Expand Down Expand Up @@ -210,7 +226,15 @@
}
}

#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
#[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
fn parse_header_string(value: &str) -> impl Iterator<Item = (&str, String)> {
value
.split_terminator(',')
.map(str::trim)
.filter_map(parse_header_key_value_string)
}

#[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
fn url_decode(value: &str) -> Option<String> {
let mut result = String::with_capacity(value.len());
let mut chars_to_decode = Vec::<u8>::new();
Expand Down Expand Up @@ -240,15 +264,7 @@
}
}

#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
fn parse_header_string(value: &str) -> impl Iterator<Item = (&str, String)> {
value
.split_terminator(',')
.map(str::trim)
.filter_map(parse_header_key_value_string)
}

#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
#[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
fn parse_header_key_value_string(key_value_string: &str) -> Option<(&str, String)> {
key_value_string
.split_once('=')
Expand All @@ -262,9 +278,8 @@
}

#[cfg(test)]
#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
#[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
mod tests {

pub(crate) fn run_env_test<T, F>(env_vars: T, f: F)
where
F: FnOnce(),
Expand All @@ -280,7 +295,7 @@
)
}

#[cfg(feature = "http-proto")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
#[test]
fn test_default_http_endpoint() {
let exporter_builder = crate::new_exporter().http();
Expand All @@ -302,6 +317,39 @@
);
}

#[test]
fn test_default_protocol() {
#[cfg(all(
feature = "http-json",
not(all(feature = "grpc-tonic", feature = "http-proto"))
))]
{
assert_eq!(
crate::exporter::default_protocol(),
crate::Protocol::HttpJson
);
}

#[cfg(all(
feature = "http-proto",
not(all(feature = "grpc-tonic", feature = "http-json"))
))]
{
assert_eq!(
crate::exporter::default_protocol(),
crate::Protocol::HttpBinary
);
}

#[cfg(all(
feature = "grpc-tonic",
not(all(feature = "http-proto", feature = "http-json"))
))]
{
assert_eq!(crate::exporter::default_protocol(), crate::Protocol::Grpc);
}
}

#[test]
fn test_url_decode() {
let test_cases = vec![
Expand Down
Loading
Loading