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

Remove support for surf HTTP client #1537

Merged
merged 2 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
14 changes: 1 addition & 13 deletions opentelemetry-jaeger/src/exporter/config/collector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@ 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.
/// - [reqwest blocking client], requires `reqwest_blocking_collector_client` feature enabled, use [`with_reqwest_blocking`][CollectorPipeline::with_reqwest_blocking] function to setup.
///
/// Additionally you can enable https
///
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 @@
#[cfg_attr(
all(
not(feature = "reqwest-client"),
not(feature = "surf-client"),
not(feature = "reqwest-blocking-client")
),
derive(Default)
Expand All @@ -40,31 +39,16 @@
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"))]

Check warning on line 48 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L48

Added line #L48 was not covered by tests
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
Loading