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

chore: make tracing optional #776

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ jobs:
- name: Check with unstable flag
run: cargo check --features unstable

- name: Check with tracing feature
run: cargo check --features tracing

- name: Run lib tests and doc tests
run: cargo test

Expand Down
7 changes: 6 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ rust-version = "1.63"
# Enables `futures::Stream` implementations for various types.
stream = []

# Enables tracing.
tracing = ["dep:tracing"]

# Enables **unstable** APIs. Any API exposed by this feature has no backwards
# compatibility guarantees. In other words, you should not use this feature for
# anything besides experimentation. Definitely **do not** publish a crate that
Expand All @@ -46,12 +49,14 @@ tokio-util = { version = "0.7.1", features = ["codec", "io"] }
tokio = { version = "1", features = ["io-util"] }
bytes = "1"
http = "1"
tracing = { version = "0.1.35", default-features = false, features = ["std"] }
tracing = { version = "0.1.35", default-features = false, features = ["std"], optional = true }
fnv = "1.0.5"
slab = "0.4.2"
indexmap = { version = "2", features = ["std"] }

[dev-dependencies]
# Test
tracing = { version = "0.1.35", default-features = false, features = ["std"] }

# Fuzzing
quickcheck = { version = "1.0.3", default-features = false }
Expand Down
17 changes: 12 additions & 5 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,10 @@ use crate::codec::{Codec, SendError, UserError};
use crate::ext::Protocol;
use crate::frame::{Headers, Pseudo, Reason, Settings, StreamId};
use crate::proto::{self, Error};
use crate::{FlowControl, PingPong, RecvStream, SendStream};
use crate::{tracing, FlowControl, PingPong, RecvStream, SendStream};

#[cfg(feature = "tracing")]
use ::tracing::Instrument;
use bytes::{Buf, Bytes};
use http::{uri, HeaderMap, Method, Request, Response, Version};
use std::fmt;
Expand All @@ -149,7 +151,6 @@ use std::pin::Pin;
use std::task::{Context, Poll};
use std::time::Duration;
use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt};
use tracing::Instrument;

/// Initializes new HTTP/2 streams on a connection by sending a request.
///
Expand Down Expand Up @@ -1275,10 +1276,15 @@ where
T: AsyncRead + AsyncWrite + Unpin,
{
let builder = Builder::new();
builder

#[cfg(feature = "tracing")]
return builder
.handshake(io)
.instrument(tracing::trace_span!("client_handshake"))
.await
.instrument(::tracing::trace_span!("client_handshake"))
.await;

#[cfg(not(feature = "tracing"))]
return builder.handshake(io).await;
}

// ===== impl Connection =====
Expand Down Expand Up @@ -1646,6 +1652,7 @@ impl Peer {
impl proto::Peer for Peer {
type Poll = Response<()>;

#[cfg(feature = "tracing")]
const NAME: &'static str = "Client";

fn r#dyn() -> proto::DynPeer {
Expand Down
47 changes: 23 additions & 24 deletions src/codec/framed_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::frame::{
use crate::proto::Error;

use crate::hpack;
use crate::tracing;

use futures_core::Stream;

Expand Down Expand Up @@ -126,8 +127,7 @@ fn decode_frame(
partial_inout: &mut Option<Partial>,
mut bytes: BytesMut,
) -> Result<Option<Frame>, Error> {
let span = tracing::trace_span!("FramedRead::decode_frame", offset = bytes.len());
let _e = span.enter();
let _span = tracing::trace_span!("FramedRead::decode_frame", offset = bytes.len());

tracing::trace!("decoding frame from {}B", bytes.len());

Expand Down Expand Up @@ -159,8 +159,8 @@ fn decode_frame(
// `PROTOCOL_ERROR`.
return Err(Error::library_reset($head.stream_id(), Reason::PROTOCOL_ERROR));
},
Err(e) => {
proto_err!(conn: "failed to load frame; err={:?}", e);
Err(_e) => {
proto_err!(conn: "failed to load frame; err={:?}", _e);
return Err(Error::library_go_away(Reason::PROTOCOL_ERROR));
}
};
Expand All @@ -176,8 +176,8 @@ fn decode_frame(
proto_err!(stream: "malformed header block; stream={:?}", id);
return Err(Error::library_reset(id, Reason::PROTOCOL_ERROR));
},
Err(e) => {
proto_err!(conn: "failed HPACK decoding; err={:?}", e);
Err(_e) => {
proto_err!(conn: "failed HPACK decoding; err={:?}", _e);
return Err(Error::library_go_away(Reason::PROTOCOL_ERROR));
}
}
Expand All @@ -202,26 +202,26 @@ fn decode_frame(
Kind::Settings => {
let res = frame::Settings::load(head, &bytes[frame::HEADER_LEN..]);

res.map_err(|e| {
proto_err!(conn: "failed to load SETTINGS frame; err={:?}", e);
res.map_err(|_e| {
proto_err!(conn: "failed to load SETTINGS frame; err={:?}", _e);
Error::library_go_away(Reason::PROTOCOL_ERROR)
})?
.into()
}
Kind::Ping => {
let res = frame::Ping::load(head, &bytes[frame::HEADER_LEN..]);

res.map_err(|e| {
proto_err!(conn: "failed to load PING frame; err={:?}", e);
res.map_err(|_e| {
proto_err!(conn: "failed to load PING frame; err={:?}", _e);
Error::library_go_away(Reason::PROTOCOL_ERROR)
})?
.into()
}
Kind::WindowUpdate => {
let res = frame::WindowUpdate::load(head, &bytes[frame::HEADER_LEN..]);

res.map_err(|e| {
proto_err!(conn: "failed to load WINDOW_UPDATE frame; err={:?}", e);
res.map_err(|_e| {
proto_err!(conn: "failed to load WINDOW_UPDATE frame; err={:?}", _e);
Error::library_go_away(Reason::PROTOCOL_ERROR)
})?
.into()
Expand All @@ -231,25 +231,25 @@ fn decode_frame(
let res = frame::Data::load(head, bytes.freeze());

// TODO: Should this always be connection level? Probably not...
res.map_err(|e| {
proto_err!(conn: "failed to load DATA frame; err={:?}", e);
res.map_err(|_e| {
proto_err!(conn: "failed to load DATA frame; err={:?}", _e);
Error::library_go_away(Reason::PROTOCOL_ERROR)
})?
.into()
}
Kind::Headers => header_block!(Headers, head, bytes),
Kind::Reset => {
let res = frame::Reset::load(head, &bytes[frame::HEADER_LEN..]);
res.map_err(|e| {
proto_err!(conn: "failed to load RESET frame; err={:?}", e);
res.map_err(|_e| {
proto_err!(conn: "failed to load RESET frame; err={:?}", _e);
Error::library_go_away(Reason::PROTOCOL_ERROR)
})?
.into()
}
Kind::GoAway => {
let res = frame::GoAway::load(&bytes[frame::HEADER_LEN..]);
res.map_err(|e| {
proto_err!(conn: "failed to load GO_AWAY frame; err={:?}", e);
res.map_err(|_e| {
proto_err!(conn: "failed to load GO_AWAY frame; err={:?}", _e);
Error::library_go_away(Reason::PROTOCOL_ERROR)
})?
.into()
Expand All @@ -272,8 +272,8 @@ fn decode_frame(
proto_err!(stream: "PRIORITY invalid dependency ID; stream={:?}", id);
return Err(Error::library_reset(id, Reason::PROTOCOL_ERROR));
}
Err(e) => {
proto_err!(conn: "failed to load PRIORITY frame; err={:?};", e);
Err(_e) => {
proto_err!(conn: "failed to load PRIORITY frame; err={:?};", _e);
return Err(Error::library_go_away(Reason::PROTOCOL_ERROR));
}
}
Expand Down Expand Up @@ -348,8 +348,8 @@ fn decode_frame(
proto_err!(stream: "malformed CONTINUATION frame; stream={:?}", id);
return Err(Error::library_reset(id, Reason::PROTOCOL_ERROR));
}
Err(e) => {
proto_err!(conn: "failed HPACK decoding; err={:?}", e);
Err(_e) => {
proto_err!(conn: "failed HPACK decoding; err={:?}", _e);
return Err(Error::library_go_away(Reason::PROTOCOL_ERROR));
}
}
Expand Down Expand Up @@ -377,8 +377,7 @@ where
type Item = Result<Frame, Error>;

fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
let span = tracing::trace_span!("FramedRead::poll_next");
let _e = span.enter();
let _span = tracing::trace_span!("FramedRead::poll_next");
loop {
tracing::trace!("poll");
let bytes = match ready!(Pin::new(&mut self.inner).poll_next(cx)) {
Expand Down
8 changes: 3 additions & 5 deletions src/codec/framed_write.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::codec::UserError;
use crate::codec::UserError::*;
use crate::frame::{self, Frame, FrameSize};
use crate::hpack;
use crate::{hpack, tracing};

use bytes::{Buf, BufMut, BytesMut};
use std::pin::Pin;
Expand Down Expand Up @@ -128,8 +128,7 @@ where

/// Flush buffered data to the wire
pub fn flush(&mut self, cx: &mut Context) -> Poll<io::Result<()>> {
let span = tracing::trace_span!("FramedWrite::flush");
let _e = span.enter();
let _span = tracing::trace_span!("FramedWrite::flush");

loop {
while !self.encoder.is_empty() {
Expand Down Expand Up @@ -207,8 +206,7 @@ where
fn buffer(&mut self, item: Frame<B>) -> Result<(), UserError> {
// Ensure that we have enough capacity to accept the write.
assert!(self.has_capacity());
let span = tracing::trace_span!("FramedWrite::buffer", frame = ?item);
let _e = span.enter();
let _span = tracing::trace_span!("FramedWrite::buffer", frame = ?item);

tracing::debug!(frame = ?item, "send");

Expand Down
1 change: 1 addition & 0 deletions src/frame/go_away.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::fmt;
use bytes::{BufMut, Bytes};

use crate::frame::{self, Error, Head, Kind, Reason, StreamId};
use crate::tracing;

#[derive(Clone, Eq, PartialEq)]
pub struct GoAway {
Expand Down
1 change: 1 addition & 0 deletions src/frame/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::{util, StreamDependency, StreamId};
use crate::ext::Protocol;
use crate::frame::{Error, Frame, Head, Kind};
use crate::hpack::{self, BytesStr};
use crate::tracing;

use http::header::{self, HeaderName, HeaderValue};
use http::{uri, HeaderMap, Method, Request, StatusCode, Uri};
Expand Down
1 change: 1 addition & 0 deletions src/frame/ping.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::frame::{Error, Frame, Head, Kind, StreamId};
use crate::tracing;
use bytes::BufMut;

const ACK_FLAG: u8 = 0x1;
Expand Down
1 change: 1 addition & 0 deletions src/frame/reset.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::frame::{self, Error, Head, Kind, Reason, StreamId};
use crate::tracing;

use bytes::BufMut;

Expand Down
1 change: 1 addition & 0 deletions src/frame/settings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fmt;

use crate::frame::{util, Error, Frame, FrameSize, Head, Kind, StreamId};
use crate::tracing;
use bytes::{BufMut, BytesMut};

#[derive(Clone, Default, Eq, PartialEq)]
Expand Down
1 change: 1 addition & 0 deletions src/frame/window_update.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::frame::{self, Error, Head, Kind, StreamId};
use crate::tracing;

use bytes::BufMut;

Expand Down
6 changes: 3 additions & 3 deletions src/hpack/decoder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{header::BytesStr, huffman, Header};
use crate::frame;
use crate::{frame, tracing};

use bytes::{Buf, Bytes, BytesMut};
use http::header;
Expand Down Expand Up @@ -189,8 +189,7 @@ impl Decoder {
self.last_max_update = size;
}

let span = tracing::trace_span!("hpack::decode");
let _e = span.enter();
let _span = tracing::trace_span!("hpack::decode");

tracing::trace!("decode");

Expand Down Expand Up @@ -494,6 +493,7 @@ impl Table {
}
}

#[cfg(any(feature = "tracing", test))]
fn size(&self) -> usize {
self.size
}
Expand Down
4 changes: 2 additions & 2 deletions src/hpack/encoder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::table::{Index, Table};
use super::{huffman, Header};
use crate::tracing;

use bytes::{BufMut, BytesMut};
use http::header::{HeaderName, HeaderValue};
Expand Down Expand Up @@ -62,8 +63,7 @@ impl Encoder {
where
I: IntoIterator<Item = Header<Option<HeaderName>>>,
{
let span = tracing::trace_span!("hpack::encode");
let _e = span.enter();
let _span = tracing::trace_span!("hpack::encode");

self.encode_size_updates(dst);

Expand Down
6 changes: 4 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,14 @@
#![allow(clippy::type_complexity, clippy::manual_range_contains)]
#![cfg_attr(test, deny(warnings))]

mod tracing;

macro_rules! proto_err {
(conn: $($msg:tt)+) => {
tracing::debug!("connection error PROTOCOL_ERROR -- {};", format_args!($($msg)+))
crate::tracing::debug!("connection error PROTOCOL_ERROR -- {};", format_args!($($msg)+))
};
(stream: $($msg:tt)+) => {
tracing::debug!("stream error PROTOCOL_ERROR -- {};", format_args!($($msg)+))
crate::tracing::debug!("stream error PROTOCOL_ERROR -- {};", format_args!($($msg)+))
};
}

Expand Down
Loading