Skip to content

Commit

Permalink
chore: make tracing optional
Browse files Browse the repository at this point in the history
  • Loading branch information
tottoto committed Sep 12, 2024
1 parent 77be664 commit c8a2b13
Show file tree
Hide file tree
Showing 29 changed files with 233 additions and 132 deletions.
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

0 comments on commit c8a2b13

Please sign in to comment.