Skip to content

Commit

Permalink
Remove support for surf HTTP client
Browse files Browse the repository at this point in the history
  • Loading branch information
djc committed Feb 16, 2024
1 parent d96d59f commit a91ce8c
Show file tree
Hide file tree
Showing 22 changed files with 18 additions and 182 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ rand = "0.8"
reqwest = "0.11"
serde = "1.0"
serde_json = "1.0"
surf = "2.0"
temp-env = "0.3.6"
thiserror = "1"
tonic = "0.11"
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

### Changed

- **Breaking** Remove built-in support for surf HTTP client ([#1537](https://github.com/open-telemetry/opentelemetry-rust/pull/1537))
- **Breaking** Surface non-2xx status codes as errors; change `ResponseExt` trait to return `HttpError` instead of `TraceError`[#1484](https://github.com/open-telemetry/opentelemetry-rust/pull/1484)



## v0.10.0

### Changed
Expand Down
1 change: 0 additions & 1 deletion opentelemetry-http/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@ hyper = { workspace = true, features = ["http2", "client", "tcp"], optional = tr
isahc = { workspace = true, optional = true }
opentelemetry = { version = "0.21", path = "../opentelemetry", features = ["trace"] }
reqwest = { workspace = true, features = ["blocking"], optional = true }
surf = { workspace = true, optional = true }
tokio = { workspace = true, features = ["time"], optional = true }
60 changes: 0 additions & 60 deletions opentelemetry-http/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,66 +100,6 @@ mod reqwest {
}
}

#[cfg(feature = "surf")]
pub mod surf {
use std::str::FromStr;

use http::{header::HeaderName, HeaderMap, HeaderValue};

use super::{async_trait, Bytes, HttpClient, HttpError, Request, Response, ResponseExt};

#[derive(Debug)]
pub struct BasicAuthMiddleware(pub surf::http::auth::BasicAuth);

#[async_trait]
impl surf::middleware::Middleware for BasicAuthMiddleware {
async fn handle(
&self,
mut req: surf::Request,
client: surf::Client,
next: surf::middleware::Next<'_>,
) -> surf::Result<surf::Response> {
req.insert_header(self.0.name(), self.0.value());
next.run(req, client).await
}
}

#[async_trait]
impl HttpClient for surf::Client {
async fn send(&self, request: Request<Vec<u8>>) -> Result<Response<Bytes>, HttpError> {
let (parts, body) = request.into_parts();
let method = parts.method.as_str().parse()?;
let uri = parts.uri.to_string().parse()?;

let mut request_builder = surf::Request::builder(method, uri).body(body);
let mut prev_name = None;
for (new_name, value) in parts.headers.into_iter() {
let name = new_name.or(prev_name).expect("the first time new_name should be set and from then on we always have a prev_name");
request_builder = request_builder.header(name.as_str(), value.to_str()?);
prev_name = Some(name);
}

let mut response = self.send(request_builder).await?;
let mut headers = HeaderMap::new();
for header_name in response.header_names() {
for header_value in &response[header_name.to_string().as_str()] {
headers.append(
HeaderName::from_str(&header_name.to_string())?,
HeaderValue::from_str(header_value.as_str())?,
);
}
}
let mut http_response = Response::builder()
.status(response.status() as u16)
.body(response.body_bytes().await?.into())?;

*http_response.headers_mut() = headers;

Ok(http_response.error_for_status()?)
}
}
}

#[cfg(feature = "isahc")]
mod isahc {
use crate::ResponseExt;
Expand Down
1 change: 1 addition & 0 deletions opentelemetry-jaeger/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Changed

- **Breaking** Remove support for surf HTTP client ([#1537](https://github.com/open-telemetry/opentelemetry-rust/pull/1537))
- Update to tonic 0.11 and prost 0.12 (#1536)

## v0.21.0
Expand Down
5 changes: 1 addition & 4 deletions opentelemetry-jaeger/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ opentelemetry-http = { version = "0.10", path = "../opentelemetry-http", optiona
opentelemetry-semantic-conventions = { version = "0.13", path = "../opentelemetry-semantic-conventions" }
pin-project-lite = { workspace = true, optional = true }
reqwest = { workspace = true, optional = true }
surf = { workspace = true, optional = true }
thrift = "0.17.0"
tokio = { workspace = true, features = ["net", "sync"], optional = true }
wasm-bindgen = { version = "0.2", optional = true }
Expand Down Expand Up @@ -79,7 +78,6 @@ full = [
"reqwest_collector_client",
"reqwest_blocking_collector_client",
"reqwest_rustls_collector_client",
"surf_collector_client",
"wasm_collector_client",
"rt-tokio",
"rt-tokio-current-thread",
Expand All @@ -94,7 +92,6 @@ isahc_collector_client = ["isahc", "opentelemetry-http/isahc"]
reqwest_blocking_collector_client = ["reqwest/blocking", "collector_client", "headers", "opentelemetry-http/reqwest"]
reqwest_collector_client = ["reqwest", "collector_client", "headers", "opentelemetry-http/reqwest"]
reqwest_rustls_collector_client = ["reqwest_collector_client", "reqwest/rustls-tls-native-roots"]
surf_collector_client = ["surf", "collector_client", "opentelemetry-http/surf"]
wasm_collector_client = [
"base64",
"http",
Expand All @@ -107,4 +104,4 @@ wasm_collector_client = [
rt-tokio = ["tokio", "opentelemetry_sdk/rt-tokio"]
rt-tokio-current-thread = ["tokio", "opentelemetry_sdk/rt-tokio-current-thread"]
rt-async-std = ["async-std", "opentelemetry_sdk/rt-async-std"]
integration_test = ["tonic", "prost", "prost-types", "rt-tokio", "collector_client", "hyper_collector_client", "hyper_tls_collector_client", "reqwest_collector_client", "surf_collector_client", "isahc_collector_client"]
integration_test = ["tonic", "prost", "prost-types", "rt-tokio", "collector_client", "hyper_collector_client", "hyper_tls_collector_client", "reqwest_collector_client", "isahc_collector_client"]
1 change: 0 additions & 1 deletion opentelemetry-jaeger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ Then you can use the [`with_collector_endpoint`] method to specify the endpoint:
```rust
// Note that this requires one of the following features enabled so that there is a default http client implementation
// * hyper_collector_client
// * surf_collector_client
// * reqwest_collector_client
// * reqwest_blocking_collector_client
// * reqwest_rustls_collector_client
Expand Down
26 changes: 0 additions & 26 deletions opentelemetry-jaeger/src/exporter/config/collector/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ pub(crate) enum CollectorHttpClient {
Hyper,
#[cfg(feature = "isahc_collector_client")]
Isahc,
#[cfg(feature = "surf_collector_client")]
Surf,
#[cfg(feature = "reqwest_collector_client")]
Reqwest,
#[cfg(feature = "reqwest_blocking_collector_client")]
Expand Down Expand Up @@ -57,30 +55,6 @@ impl CollectorHttpClient {
})?;
Ok(Box::new(client))
}
#[cfg(feature = "surf_collector_client")]
CollectorHttpClient::Surf => {
use opentelemetry_http::surf::BasicAuthMiddleware;

let client: surf::Client = surf::Config::new()
.set_timeout(Some(collector_timeout))
.try_into()
.map_err(|err| crate::Error::ConfigError {
pipeline_name: "collector",
config_name: "http_client",
reason: format!("cannot create surf client. {}", err),
})?;

let client = if let (Some(username), Some(password)) =
(collector_username, collector_password)
{
let auth = surf::http::auth::BasicAuth::new(username, password);
client.with(BasicAuthMiddleware(auth))
} else {
client
};

Ok(Box::new(client))
}
#[cfg(feature = "reqwest_blocking_collector_client")]
CollectorHttpClient::ReqwestBlocking => {
use headers::authorization::Credentials;
Expand Down
12 changes: 0 additions & 12 deletions opentelemetry-jaeger/src/exporter/config/collector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ const ENV_PASSWORD: &str = "OTEL_EXPORTER_JAEGER_PASSWORD";
/// implementation and relative configurations.
///
/// - [hyper], requires `hyper_collector_client` feature enabled, use [`with_hyper`][CollectorPipeline::with_hyper] function to setup.
/// - [surf], requires `surf_collector_client` feature enabled, use [`with_surf`][CollectorPipeline::with_surf] function to setup.
/// - [isahc], requires `isahc_collector_client` feature enabled, use [`with_isahc`][CollectorPipeline::with_isahc] function to setup.
/// - [reqwest], requires `reqwest_collector_client` feature enabled, use [`with_reqwest`][CollectorPipeline::with_reqwest] function to setup.
/// - [reqwest blocking client], requires `reqwest_blocking_collector_client` feature enabled, use [`with_reqwest_blocking`][CollectorPipeline::with_surf] function to setup.
Expand Down Expand Up @@ -277,17 +276,6 @@ impl CollectorPipeline {
}
}

/// Use surf http client in the exporter.
#[cfg(feature = "surf_collector_client")]
pub fn with_surf(self) -> Self {
Self {
client_config: ClientConfig::Http {
client_type: CollectorHttpClient::Surf,
},
..self
}
}

/// Use reqwest http client in the exporter.
#[cfg(feature = "reqwest_collector_client")]
pub fn with_reqwest(self) -> Self {
Expand Down
2 changes: 0 additions & 2 deletions opentelemetry-jaeger/src/exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
//!
// Linting isn't detecting that it's used seems like linting bug.
#[allow(unused_imports)]
#[cfg(feature = "surf_collector_client")]
use std::convert::TryFrom;
use std::convert::TryInto;
use std::fmt::Display;
use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr};
Expand Down
5 changes: 0 additions & 5 deletions opentelemetry-jaeger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,6 @@
//! .with_resource(Resource::new(vec![KeyValue::new("key", "value"),
//! KeyValue::new("process_key", "process_value")])),
//! )
//! // we config a surf http client with 2 seconds timeout
//! // and have basic authentication header with username=username, password=s3cr3t
//! .with_isahc() // requires `isahc_collector_client` feature
//! .with_username("username")
//! .with_password("s3cr3t")
//! .with_timeout(std::time::Duration::from_secs(2))
Expand All @@ -259,8 +256,6 @@
//!
//! * `hyper_collector_client`: Export span data with Jaeger collector backed by a hyper default http client.
//!
//! * `surf_collector_client`: Export span data with Jaeger collector backed by a surf default http client.
//!
//! * `reqwest_collector_client`: Export span data with Jaeger collector backed by a reqwest http client.
//!
//! * `reqwest_blocking_collector_client`: Export span data with Jaeger collector backed by a reqwest blocking http client.
Expand Down
11 changes: 0 additions & 11 deletions opentelemetry-jaeger/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,6 @@ mod tests {
.expect("cannot create tracer using default configuration")
}),
),
(
"collector_surf",
Box::new(|| {
opentelemetry_jaeger::new_collector_pipeline()
.with_endpoint(collector_endpoint)
.with_surf()
.with_service_name(format!("{}-{}", SERVICE_NAME, "collector_surf"))
.install_batch(opentelemetry_sdk::runtime::Tokio)
.expect("cannot create tracer using default configuration")
}),
),
(
"collector_hyper",
Box::new(|| {
Expand Down
1 change: 1 addition & 0 deletions opentelemetry-otlp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## vNext

- **Breaking** Remove support for surf HTTP client ([#1537](https://github.com/open-telemetry/opentelemetry-rust/pull/1537))
- Update to tonic 0.11 and prost 0.12 (#1536)
- Remove support for grpcio transport (#1534)

Expand Down
2 changes: 0 additions & 2 deletions opentelemetry-otlp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ tonic = { workspace = true, optional = true }
tokio = { workspace = true, features = ["sync", "rt"], optional = true }

reqwest = { workspace = true, optional = true }
surf = { workspace = true, optional = true }
http = { workspace = true, optional = true }
serde = { workspace = true, features = ["derive"], optional = true }
thiserror = { workspace = true }
Expand Down Expand Up @@ -74,7 +73,6 @@ 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"]
surf-client = ["surf", "opentelemetry-http/surf"]

# test
integration-testing = ["tonic", "prost", "tokio/full", "trace"]
20 changes: 2 additions & 18 deletions opentelemetry-otlp/src/exporter/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ mod trace;
#[cfg_attr(
all(
not(feature = "reqwest-client"),
not(feature = "surf-client"),
not(feature = "reqwest-blocking-client")
),
derive(Default)
Expand All @@ -40,31 +39,16 @@ pub(crate) struct HttpConfig {
headers: Option<HashMap<String, String>>,
}

#[cfg(any(
feature = "reqwest-blocking-client",
feature = "reqwest-client",
feature = "surf-client"
))]
#[cfg(any(feature = "reqwest-blocking-client", feature = "reqwest-client",))]
impl Default for HttpConfig {
fn default() -> Self {
HttpConfig {
#[cfg(feature = "reqwest-blocking-client")]
client: Some(Arc::new(reqwest::blocking::Client::new())),
#[cfg(all(
not(feature = "reqwest-blocking-client"),
not(feature = "surf-client"),
feature = "reqwest-client"
))]
#[cfg(all(not(feature = "reqwest-blocking-client"), feature = "reqwest-client"))]
client: Some(Arc::new(reqwest::Client::new())),
#[cfg(all(
not(feature = "reqwest-client"),
not(feature = "reqwest-blocking-client"),
feature = "surf-client"
))]
client: Some(Arc::new(surf::Client::new())),
#[cfg(all(
not(feature = "reqwest-client"),
not(feature = "surf-client"),
not(feature = "reqwest-blocking-client")
))]
client: None,
Expand Down
2 changes: 0 additions & 2 deletions opentelemetry-otlp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@
//! * `reqwest-blocking-client`: Use reqwest blocking http client.
//! * `reqwest-client`: Use reqwest http client.
//! * `reqwest-rustls`: Use reqwest with TLS.
//! * `surf-client`: Use surf http client.
//!
//!
//! # Kitchen Sink Full Configuration
//!
Expand Down
4 changes: 4 additions & 0 deletions opentelemetry-zipkin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## vNext

### Changed

- **Breaking** Remove support for surf HTTP client ([#1537](https://github.com/open-telemetry/opentelemetry-rust/pull/1537))

## v0.19.0

### Changed
Expand Down
2 changes: 0 additions & 2 deletions opentelemetry-zipkin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ default = ["reqwest-blocking-client", "reqwest/native-tls"]
reqwest-blocking-client = ["reqwest/blocking", "opentelemetry-http/reqwest"]
reqwest-client = ["reqwest", "opentelemetry-http/reqwest"]
reqwest-rustls = ["reqwest", "reqwest/rustls-tls-native-roots"]
surf-client = ["surf", "opentelemetry-http/surf"]

[dependencies]
async-trait = { workspace = true }
Expand All @@ -38,7 +37,6 @@ serde = { workspace = true, features = ["derive"] }
typed-builder = "0.12"
http = { workspace = true }
reqwest = { workspace = true, optional = true}
surf = { workspace = true, optional = true}
thiserror = { workspace = true }
futures-core = { workspace = true }

Expand Down
7 changes: 3 additions & 4 deletions opentelemetry-zipkin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
tracer.in_span("doing_work", |cx| {
// Traced app logic here...
});

global::shutdown_tracer_provider();

Ok(())
Expand Down Expand Up @@ -86,9 +86,8 @@ a manual implementation of the [`HttpClient`] trait. By default the
`reqwest-blocking-client` feature is enabled which will use the `reqwest` crate.
While this is compatible with both async and non-async projects, it is not
optimal for high-performance async applications as it will block the executor
thread. Consider using the `reqwest-client` (without blocking) or `surf-client`
features if you are in the `tokio` or `async-std` ecosystems respectively, or
select whichever client you prefer as shown below.
thread. Consider using the `reqwest-client` (without blocking) if you are in
the `tokio` ecosystem.

Note that async http clients may require a specific async runtime to be
available so be sure to match them appropriately.
Expand Down
Loading

0 comments on commit a91ce8c

Please sign in to comment.