From f06a5be6b3f617513c5c02e676cb878a6638a468 Mon Sep 17 00:00:00 2001 From: Jordan Doyle Date: Sat, 19 Mar 2022 01:56:38 +0000 Subject: [PATCH] Instrument all the things --- Cargo.toml | 1 + src/git_command_handlers/fetch.rs | 2 ++ src/git_command_handlers/ls_refs.rs | 2 ++ src/main.rs | 18 ++++++++++++++---- src/protocol/codec.rs | 2 ++ src/protocol/high_level.rs | 4 ++++ src/protocol/low_level.rs | 8 ++++++++ src/protocol/packet_line.rs | 2 ++ src/providers/gitlab.rs | 14 ++++++++++---- src/providers/mod.rs | 4 ++-- 10 files changed, 47 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a9eed18..9c54bbf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,3 +42,4 @@ toml = "0.5" url = { version = "2.2", features = ["serde"] } ustr = "0.8" uuid = { version = "1.0.0-alpha.1", features = ["v4"] } + diff --git a/src/git_command_handlers/fetch.rs b/src/git_command_handlers/fetch.rs index 583ec5c..98ad613 100644 --- a/src/git_command_handlers/fetch.rs +++ b/src/git_command_handlers/fetch.rs @@ -1,5 +1,6 @@ use bytes::Bytes; use thrussh::{server::Session, ChannelId}; +use tracing::instrument; use crate::{ protocol::{ @@ -9,6 +10,7 @@ use crate::{ Handler, PackageProvider, UserProvider, }; +#[instrument(skip(handle, session, channel, metadata, packfile_entries), err)] pub fn handle( handle: &mut Handler, session: &mut Session, diff --git a/src/git_command_handlers/ls_refs.rs b/src/git_command_handlers/ls_refs.rs index 3c67aa6..1ae3521 100644 --- a/src/git_command_handlers/ls_refs.rs +++ b/src/git_command_handlers/ls_refs.rs @@ -6,12 +6,14 @@ use bytes::Bytes; use thrussh::{server::Session, ChannelId}; +use tracing::instrument; use crate::{ protocol::{low_level::HashOutput, packet_line::PktLine}, Handler, PackageProvider, UserProvider, }; +#[instrument(skip(handle, session, channel, _metadata, commit_hash), err)] pub fn handle( handle: &mut Handler, session: &mut Session, diff --git a/src/main.rs b/src/main.rs index 242f407..54ca331 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,7 +36,7 @@ use thrussh::{ }; use thrussh_keys::key::PublicKey; use tokio_util::{codec::Decoder, codec::Encoder as CodecEncoder}; -use tracing::{debug, error, info, info_span, Instrument, Span}; +use tracing::{debug, error, info, info_span, instrument, Instrument, Span}; use uuid::Uuid; const AGENT: &str = concat!( @@ -49,7 +49,10 @@ const AGENT: &str = concat!( #[tokio::main] async fn main() -> anyhow::Result<()> { - tracing_subscriber::fmt::init(); + let subscriber = tracing_subscriber::fmt(); + #[cfg(debug_assertions)] + let subscriber = subscriber.pretty(); + subscriber.init(); let args: Args = Args::parse(); @@ -162,11 +165,15 @@ pub struct Handler { impl Handler { fn user(&self) -> anyhow::Result<&Arc> { - self.user.as_ref().ok_or_else(|| anyhow::anyhow!("no user set")) + self.user + .as_ref() + .ok_or_else(|| anyhow::anyhow!("no user set")) } fn group(&self) -> anyhow::Result<&Group> { - self.group.as_ref().ok_or_else(|| anyhow::anyhow!("no group set")) + self.group + .as_ref() + .ok_or_else(|| anyhow::anyhow!("no group set")) } /// Writes a Git packet line response to the buffer, this should only @@ -185,6 +192,7 @@ impl Handler { /// Fetches all the releases from the provider for the given group /// and groups them by crate. + #[instrument(skip(self), err)] async fn fetch_releases_by_crate( &self, ) -> anyhow::Result>> { @@ -209,6 +217,7 @@ impl Handler { /// globally cache-able since it's immutable, to get to this call /// the user must've already fetched the crate path from the provider /// and hence verified they have permission to read it. + #[instrument(skip(self), err)] async fn fetch_metadata( &self, path: &U::CratePath, @@ -262,6 +271,7 @@ impl Handler { // 1. the client receives the expected refs when calling `fetch`, // 2. we don't do the relatively expensive processing that comes with // generating the packfile more than once per connection. + #[instrument(skip(self), err)] async fn build_packfile(&mut self) -> anyhow::Result)>> { // return the cached value if we've generated the packfile for // this connection already diff --git a/src/protocol/codec.rs b/src/protocol/codec.rs index c9671b3..50ef1aa 100644 --- a/src/protocol/codec.rs +++ b/src/protocol/codec.rs @@ -2,6 +2,7 @@ use bytes::{Buf, Bytes, BytesMut}; use tokio_util::codec; +use tracing::instrument; use super::packet_line::PktLine; @@ -31,6 +32,7 @@ impl codec::Decoder for GitCodec { type Item = GitCommand; type Error = anyhow::Error; + #[instrument(skip(self, src), err)] fn decode(&mut self, src: &mut bytes::BytesMut) -> Result, Self::Error> { loop { if src.len() < 4 { diff --git a/src/protocol/high_level.rs b/src/protocol/high_level.rs index 58ab82b..13fd764 100644 --- a/src/protocol/high_level.rs +++ b/src/protocol/high_level.rs @@ -6,6 +6,7 @@ //! for our purposes because `cargo` will `git pull --force` from our Git //! server, allowing us to ignore any history the client may have. +use crate::instrument; use crate::util::ArcOrCowStr; use bytes::Bytes; use indexmap::IndexMap; @@ -33,6 +34,7 @@ impl GitRepository { /// Inserts a file into the repository, writing a file to the path /// `path/to/my-file` would require a `path` of `["path", "to"]` /// and a `file` of `"my-file"`. + #[instrument(skip(self, file, content), err)] pub fn insert( &mut self, path: &[&'static str], @@ -79,6 +81,7 @@ impl GitRepository { /// Finalises this `GitRepository` by writing a commit to the `packfile_entries`, /// all the files currently in the `tree`, returning all the packfile entries /// and also the commit hash so it can be referred to by `ls-ref`s. + #[instrument(skip(self, name, email, message), err)] pub fn commit( mut self, name: &'static str, @@ -123,6 +126,7 @@ impl Tree { /// Recursively writes the the whole tree out to the given `pack_file`, /// the tree contains pointers to (hashes of) files contained within a /// directory, and pointers to other directories. + #[instrument(skip(self, pack_file), err)] fn into_packfile_entries( self, pack_file: &mut IndexMap, diff --git a/src/protocol/low_level.rs b/src/protocol/low_level.rs index 4f19623..4179faa 100644 --- a/src/protocol/low_level.rs +++ b/src/protocol/low_level.rs @@ -7,6 +7,7 @@ use std::{ fmt::{Display, Formatter, Write}, io::Write as IoWrite, }; +use tracing::instrument; pub type HashOutput = [u8; 20]; @@ -36,6 +37,7 @@ impl<'a> PackFile<'a> { 20 } + #[instrument(skip(self, original_buf), err)] pub fn encode_to(&self, original_buf: &mut BytesMut) -> Result<(), anyhow::Error> { let mut buf = original_buf.split_off(original_buf.len()); buf.reserve(Self::header_size() + Self::footer_size()); @@ -70,6 +72,7 @@ pub struct Commit { } impl Commit { + #[instrument(skip(self, out), err)] fn encode_to(&self, out: &mut BytesMut) -> Result<(), anyhow::Error> { let mut tree_hex = [0_u8; 20 * 2]; hex::encode_to_slice(self.tree, &mut tree_hex)?; @@ -155,6 +158,7 @@ pub struct TreeItem { // `[mode] [name]\0[hash]` impl TreeItem { + #[instrument(skip(self, out), err)] fn encode_to(&self, out: &mut BytesMut) -> Result<(), anyhow::Error> { out.write_str(self.kind.mode())?; write!(out, " {}\0", self.name)?; @@ -207,6 +211,7 @@ pub enum PackFileEntry { } impl PackFileEntry { + #[instrument(skip(self, buf))] fn write_header(&self, buf: &mut BytesMut) { let mut size = self.uncompressed_size(); @@ -250,6 +255,7 @@ impl PackFileEntry { } } + #[instrument(skip(self, original_out), err)] pub fn encode_to(&self, original_out: &mut BytesMut) -> Result<(), anyhow::Error> { self.write_header(original_out); // TODO: this needs space reserving for it @@ -287,6 +293,7 @@ impl PackFileEntry { Ok(()) } + #[instrument(skip(self))] #[must_use] pub fn uncompressed_size(&self) -> usize { match self { @@ -297,6 +304,7 @@ impl PackFileEntry { } // wen const generics for RustCrypto? :-( + #[instrument(skip(self), err)] pub fn hash(&self) -> Result { let size = self.uncompressed_size(); diff --git a/src/protocol/packet_line.rs b/src/protocol/packet_line.rs index e469001..46dd236 100644 --- a/src/protocol/packet_line.rs +++ b/src/protocol/packet_line.rs @@ -1,5 +1,6 @@ use bytes::{BufMut, BytesMut}; use std::fmt::Write; +use tracing::instrument; use super::low_level::PackFile; @@ -18,6 +19,7 @@ pub enum PktLine<'a> { } impl PktLine<'_> { + #[instrument(skip(self, buf), err)] pub fn encode_to(&self, buf: &mut BytesMut) -> Result<(), anyhow::Error> { match self { Self::Data(data) => { diff --git a/src/providers/gitlab.rs b/src/providers/gitlab.rs index 74fc63f..17a2ce8 100644 --- a/src/providers/gitlab.rs +++ b/src/providers/gitlab.rs @@ -8,7 +8,7 @@ use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; use reqwest::header; use serde::{Deserialize, Serialize}; use std::{borrow::Cow, sync::Arc}; -use tracing::Instrument; +use tracing::{info_span, instrument, Instrument}; use url::Url; pub struct Gitlab { @@ -35,6 +35,7 @@ impl Gitlab { #[async_trait] impl super::UserProvider for Gitlab { + #[instrument(skip(self, username_password), err)] async fn find_user_by_username_password_combo( &self, username_password: &str, @@ -68,6 +69,7 @@ impl super::UserProvider for Gitlab { } } + #[instrument(skip(self), err)] async fn find_user_by_ssh_key(&self, fingerprint: &str) -> anyhow::Result> { let mut url = self.base_url.join("keys")?; url.query_pairs_mut() @@ -83,6 +85,7 @@ impl super::UserProvider for Gitlab { })) } + #[instrument(skip(self), err)] async fn fetch_token_for_user(&self, user: &User) -> anyhow::Result { let impersonation_token: GitlabImpersonationTokenResponse = handle_error( self.client @@ -109,7 +112,8 @@ impl super::UserProvider for Gitlab { impl super::PackageProvider for Gitlab { type CratePath = Arc; - async fn fetch_group(self: Arc, group: &str, do_as: &User) -> anyhow::Result { + #[instrument(skip(self), err)] + async fn fetch_group(&self, group: &str, do_as: &User) -> anyhow::Result { let mut url = self .base_url .join("groups/")? @@ -126,6 +130,7 @@ impl super::PackageProvider for Gitlab { Ok(req) } + #[instrument(skip(self), err)] async fn fetch_releases_for_group( self: Arc, group: &Group, @@ -213,7 +218,7 @@ impl super::PackageProvider for Gitlab { }), ) } - .in_current_span(), + .instrument(info_span!("fetch_package_files")), )); } } @@ -225,8 +230,9 @@ impl super::PackageProvider for Gitlab { .await } + #[instrument(skip(self), err)] async fn fetch_metadata_for_release( - self: Arc, + &self, path: &Self::CratePath, version: &str, ) -> anyhow::Result { diff --git a/src/providers/mod.rs b/src/providers/mod.rs index 375eac8..271043c 100644 --- a/src/providers/mod.rs +++ b/src/providers/mod.rs @@ -21,7 +21,7 @@ pub trait PackageProvider { /// figure out the path of a package. type CratePath: std::fmt::Debug + Send + std::hash::Hash + Clone + Eq + PartialEq + Send + Sync; - async fn fetch_group(self: Arc, group: &str, do_as: &User) -> anyhow::Result; + async fn fetch_group(&self, group: &str, do_as: &User) -> anyhow::Result; async fn fetch_releases_for_group( self: Arc, @@ -30,7 +30,7 @@ pub trait PackageProvider { ) -> anyhow::Result>; async fn fetch_metadata_for_release( - self: Arc, + &self, path: &Self::CratePath, version: &str, ) -> anyhow::Result; -- libgit2 1.7.2