mirror of
https://github.com/facebook/sapling.git
synced 2024-10-11 17:27:53 +03:00
scs_server: add scuba tracing for request start and end
Summary: Extract creation of the `CoreContext` to the macro definition of each thrift method, and then use this `CoreContext` to log to scuba at the start and end of each request. In order to log whether the request failed because of a request error or because of an internal error, all method implementations need to return `ServiceError`. This works to our advantage, as it means we can remove the `Into<thrift::Error>` impls for `MononokeError` in the `mononoke_api` crate. Reviewed By: krallin Differential Revision: D19345597 fbshipit-source-id: 559be4b73b52d501803702d0dc180f7cf3136492
This commit is contained in:
Mark Thomas
Facebook Github Bot
parent
e1f60bd30c
commit
79d3db1e5f
@ -15,9 +15,6 @@ use std::sync::Arc;
|
||||
use anyhow::Error;
|
||||
use thiserror::Error;
|
||||
|
||||
use source_control::services::source_control_service as service;
|
||||
use source_control::types as thrift;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct InternalError(Arc<Error>);
|
||||
|
||||
@ -62,45 +59,3 @@ impl From<Infallible> for MononokeError {
|
||||
unreachable!()
|
||||
}
|
||||
}
|
||||
|
||||
macro_rules! impl_into_thrift_error(
|
||||
($t:ty) => {
|
||||
impl From<MononokeError> for $t {
|
||||
fn from(e: MononokeError) -> Self {
|
||||
match e {
|
||||
MononokeError::InvalidRequest(reason) => thrift::RequestError {
|
||||
kind: thrift::RequestErrorKind::INVALID_REQUEST,
|
||||
reason,
|
||||
}
|
||||
.into(),
|
||||
MononokeError::InternalError(error) => thrift::InternalError {
|
||||
reason: error.to_string(),
|
||||
backtrace: error.backtrace().map(ToString::to_string),
|
||||
}
|
||||
.into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
// Implement From<MononokeError> for source control service exceptions. This allows using ? on
|
||||
// MononokeError and have it turn into the right exception. When adding a new error to source
|
||||
// control, add it here to get this behavior for free.
|
||||
impl_into_thrift_error!(service::ListReposExn);
|
||||
impl_into_thrift_error!(service::RepoResolveBookmarkExn);
|
||||
impl_into_thrift_error!(service::RepoListBookmarksExn);
|
||||
impl_into_thrift_error!(service::RepoCreateCommitExn);
|
||||
impl_into_thrift_error!(service::CommitFileDiffsExn);
|
||||
impl_into_thrift_error!(service::CommitLookupExn);
|
||||
impl_into_thrift_error!(service::CommitInfoExn);
|
||||
impl_into_thrift_error!(service::CommitCompareExn);
|
||||
impl_into_thrift_error!(service::CommitIsAncestorOfExn);
|
||||
impl_into_thrift_error!(service::CommitFindFilesExn);
|
||||
impl_into_thrift_error!(service::CommitPathInfoExn);
|
||||
impl_into_thrift_error!(service::CommitPathBlameExn);
|
||||
impl_into_thrift_error!(service::TreeListExn);
|
||||
impl_into_thrift_error!(service::FileExistsExn);
|
||||
impl_into_thrift_error!(service::FileInfoExn);
|
||||
impl_into_thrift_error!(service::FileContentChunkExn);
|
||||
impl_into_thrift_error!(service::CommitLookupXrepoExn);
|
||||
|
||||
@ -6,6 +6,8 @@
|
||||
* directory of this source tree.
|
||||
*/
|
||||
|
||||
use std::error::Error as StdError;
|
||||
|
||||
use mononoke_api::MononokeError;
|
||||
use source_control as thrift;
|
||||
use source_control::services::source_control_service as service;
|
||||
@ -13,7 +15,6 @@ use source_control::services::source_control_service as service;
|
||||
pub(crate) enum ServiceError {
|
||||
Request(thrift::RequestError),
|
||||
Internal(thrift::InternalError),
|
||||
Mononoke(MononokeError),
|
||||
}
|
||||
|
||||
impl From<thrift::RequestError> for ServiceError {
|
||||
@ -30,7 +31,16 @@ impl From<thrift::InternalError> for ServiceError {
|
||||
|
||||
impl From<MononokeError> for ServiceError {
|
||||
fn from(e: MononokeError) -> Self {
|
||||
Self::Mononoke(e)
|
||||
match e {
|
||||
MononokeError::InvalidRequest(reason) => Self::Request(thrift::RequestError {
|
||||
kind: thrift::RequestErrorKind::INVALID_REQUEST,
|
||||
reason,
|
||||
}),
|
||||
MononokeError::InternalError(error) => Self::Internal(thrift::InternalError {
|
||||
reason: error.to_string(),
|
||||
backtrace: error.backtrace().map(ToString::to_string),
|
||||
}),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -41,7 +51,6 @@ macro_rules! impl_into_thrift_error {
|
||||
match e {
|
||||
ServiceError::Request(e) => e.into(),
|
||||
ServiceError::Internal(e) => e.into(),
|
||||
ServiceError::Mononoke(e) => e.into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -6,6 +6,7 @@
|
||||
* directory of this source tree.
|
||||
*/
|
||||
|
||||
#![feature(backtrace)]
|
||||
#![deny(unused)]
|
||||
#![type_length_limit = "2097152"]
|
||||
|
||||
@ -38,6 +39,7 @@ mod from_request;
|
||||
mod into_response;
|
||||
mod methods;
|
||||
mod monitoring;
|
||||
mod params;
|
||||
mod source_control_impl;
|
||||
mod specifiers;
|
||||
|
||||
|
||||
@ -9,14 +9,13 @@
|
||||
use std::collections::{BTreeMap, BTreeSet};
|
||||
use std::convert::{TryFrom, TryInto};
|
||||
|
||||
use context::CoreContext;
|
||||
use futures_util::{future, stream, try_join, StreamExt, TryStreamExt};
|
||||
use mononoke_api::{
|
||||
unified_diff, ChangesetContext, ChangesetSpecifier, CopyInfo, MononokeError, MononokePath,
|
||||
RepoContext,
|
||||
};
|
||||
use source_control as thrift;
|
||||
use source_control::services::source_control_service as service;
|
||||
use srserver::RequestContext;
|
||||
|
||||
use crate::commit_id::{map_commit_identities, map_commit_identity, CommitIdExt};
|
||||
use crate::errors;
|
||||
@ -32,11 +31,10 @@ impl SourceControlServiceImpl {
|
||||
/// Look up commit.
|
||||
pub(crate) async fn commit_lookup(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit: thrift::CommitSpecifier,
|
||||
params: thrift::CommitLookupParams,
|
||||
) -> Result<thrift::CommitLookupResponse, service::CommitLookupExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit))?;
|
||||
) -> Result<thrift::CommitLookupResponse, errors::ServiceError> {
|
||||
let repo = self.repo(ctx, &commit.repo)?;
|
||||
match repo
|
||||
.changeset(ChangesetSpecifier::from_request(&commit.id)?)
|
||||
@ -59,11 +57,10 @@ impl SourceControlServiceImpl {
|
||||
/// Get diff.
|
||||
pub(crate) async fn commit_file_diffs(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit: thrift::CommitSpecifier,
|
||||
params: thrift::CommitFileDiffsParams,
|
||||
) -> Result<thrift::CommitFileDiffsResponse, service::CommitFileDiffsExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit))?;
|
||||
) -> Result<thrift::CommitFileDiffsResponse, errors::ServiceError> {
|
||||
let context_lines = params.context as usize;
|
||||
|
||||
// Check the path count limit
|
||||
@ -144,11 +141,10 @@ impl SourceControlServiceImpl {
|
||||
/// Get commit info.
|
||||
pub(crate) async fn commit_info(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit: thrift::CommitSpecifier,
|
||||
params: thrift::CommitInfoParams,
|
||||
) -> Result<thrift::CommitInfo, service::CommitInfoExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit))?;
|
||||
) -> Result<thrift::CommitInfo, errors::ServiceError> {
|
||||
let (repo, changeset) = self.repo_changeset(ctx, &commit).await?;
|
||||
|
||||
async fn map_parent_identities(
|
||||
@ -193,11 +189,10 @@ impl SourceControlServiceImpl {
|
||||
/// Returns `true` if this commit is an ancestor of `other_commit`.
|
||||
pub(crate) async fn commit_is_ancestor_of(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit: thrift::CommitSpecifier,
|
||||
params: thrift::CommitIsAncestorOfParams,
|
||||
) -> Result<bool, service::CommitIsAncestorOfExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit))?;
|
||||
) -> Result<bool, errors::ServiceError> {
|
||||
let repo = self.repo(ctx, &commit.repo)?;
|
||||
let changeset_specifier = ChangesetSpecifier::from_request(&commit.id)?;
|
||||
let other_changeset_specifier = ChangesetSpecifier::from_request(¶ms.other_commit_id)?;
|
||||
@ -220,11 +215,10 @@ impl SourceControlServiceImpl {
|
||||
// Diff two commits
|
||||
pub(crate) async fn commit_compare(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit: thrift::CommitSpecifier,
|
||||
params: thrift::CommitCompareParams,
|
||||
) -> Result<thrift::CommitCompareResponse, service::CommitCompareExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit))?;
|
||||
) -> Result<thrift::CommitCompareResponse, errors::ServiceError> {
|
||||
let (repo, base_changeset) = self.repo_changeset(ctx, &commit).await?;
|
||||
|
||||
let other_changeset_id = match ¶ms.other_commit_id {
|
||||
@ -284,11 +278,10 @@ impl SourceControlServiceImpl {
|
||||
/// Returns files that match the criteria
|
||||
pub(crate) async fn commit_find_files(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit: thrift::CommitSpecifier,
|
||||
params: thrift::CommitFindFilesParams,
|
||||
) -> Result<thrift::CommitFindFilesResponse, service::CommitFindFilesExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit))?;
|
||||
) -> Result<thrift::CommitFindFilesResponse, errors::ServiceError> {
|
||||
let (_repo, changeset) = self.repo_changeset(ctx, &commit).await?;
|
||||
let limit: usize = check_range_and_convert(
|
||||
"limit",
|
||||
@ -322,11 +315,10 @@ impl SourceControlServiceImpl {
|
||||
/// Do a cross-repo lookup to see if a commit exists under a different hash in another repo
|
||||
pub(crate) async fn commit_lookup_xrepo(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit: thrift::CommitSpecifier,
|
||||
params: thrift::CommitLookupXRepoParams,
|
||||
) -> Result<thrift::CommitLookupResponse, service::CommitLookupXrepoExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit))?;
|
||||
) -> Result<thrift::CommitLookupResponse, errors::ServiceError> {
|
||||
let repo = self.repo(ctx.clone(), &commit.repo)?;
|
||||
let other_repo = self.repo(ctx, ¶ms.other_repo)?;
|
||||
match repo
|
||||
|
||||
@ -6,11 +6,10 @@
|
||||
* directory of this source tree.
|
||||
*/
|
||||
|
||||
use context::CoreContext;
|
||||
use futures_util::future;
|
||||
use mononoke_api::{ChangesetSpecifier, MononokeError, PathEntry};
|
||||
use source_control as thrift;
|
||||
use source_control::services::source_control_service as service;
|
||||
use srserver::RequestContext;
|
||||
use std::collections::{BTreeSet, HashMap};
|
||||
use std::iter::FromIterator;
|
||||
|
||||
@ -25,11 +24,10 @@ impl SourceControlServiceImpl {
|
||||
/// Returns information about the file or directory at a path in a commit.
|
||||
pub(crate) async fn commit_path_info(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit_path: thrift::CommitPathSpecifier,
|
||||
_params: thrift::CommitPathInfoParams,
|
||||
) -> Result<thrift::CommitPathInfoResponse, service::CommitPathInfoExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit_path))?;
|
||||
) -> Result<thrift::CommitPathInfoResponse, errors::ServiceError> {
|
||||
let (_repo, changeset) = self.repo_changeset(ctx, &commit_path.commit).await?;
|
||||
let path = changeset.path(&commit_path.path)?;
|
||||
let response = match path.entry().await? {
|
||||
@ -76,11 +74,10 @@ impl SourceControlServiceImpl {
|
||||
|
||||
pub(crate) async fn commit_path_blame(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
commit_path: thrift::CommitPathSpecifier,
|
||||
params: thrift::CommitPathBlameParams,
|
||||
) -> Result<thrift::CommitPathBlameResponse, service::CommitPathBlameExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&commit_path))?;
|
||||
) -> Result<thrift::CommitPathBlameResponse, errors::ServiceError> {
|
||||
let (repo, changeset) = self.repo_changeset(ctx, &commit_path.commit).await?;
|
||||
let path = changeset.path(&commit_path.path)?;
|
||||
|
||||
|
||||
@ -9,11 +9,10 @@
|
||||
use std::cmp::min;
|
||||
|
||||
use bytes::BufMut;
|
||||
use context::CoreContext;
|
||||
use futures::stream::Stream;
|
||||
use futures_preview::compat::Future01CompatExt;
|
||||
use source_control as thrift;
|
||||
use source_control::services::source_control_service as service;
|
||||
use srserver::RequestContext;
|
||||
|
||||
use crate::errors;
|
||||
use crate::from_request::check_range_and_convert;
|
||||
@ -25,11 +24,10 @@ impl SourceControlServiceImpl {
|
||||
/// Test whether a file exists.
|
||||
pub(crate) async fn file_exists(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
file: thrift::FileSpecifier,
|
||||
_params: thrift::FileExistsParams,
|
||||
) -> Result<bool, service::FileExistsExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&file))?;
|
||||
) -> Result<bool, errors::ServiceError> {
|
||||
let (_repo, file) = self.repo_file(ctx, &file).await?;
|
||||
Ok(file.is_some())
|
||||
}
|
||||
@ -37,11 +35,10 @@ impl SourceControlServiceImpl {
|
||||
/// Get file info.
|
||||
pub(crate) async fn file_info(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
file: thrift::FileSpecifier,
|
||||
_params: thrift::FileInfoParams,
|
||||
) -> Result<thrift::FileInfo, service::FileInfoExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&file))?;
|
||||
) -> Result<thrift::FileInfo, errors::ServiceError> {
|
||||
match self.repo_file(ctx, &file).await? {
|
||||
(_repo, Some(file)) => Ok(file.metadata().await?.into_response()),
|
||||
(_repo, None) => Err(errors::file_not_found(file.description()).into()),
|
||||
@ -51,11 +48,10 @@ impl SourceControlServiceImpl {
|
||||
/// Get a chunk of file content.
|
||||
pub(crate) async fn file_content_chunk(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
file: thrift::FileSpecifier,
|
||||
params: thrift::FileContentChunkParams,
|
||||
) -> Result<thrift::FileChunk, service::FileContentChunkExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&file))?;
|
||||
) -> Result<thrift::FileChunk, errors::ServiceError> {
|
||||
let offset: u64 = check_range_and_convert("offset", params.offset, 0..)?;
|
||||
let size: u64 = check_range_and_convert(
|
||||
"size",
|
||||
|
||||
@ -6,10 +6,10 @@
|
||||
* directory of this source tree.
|
||||
*/
|
||||
|
||||
use context::CoreContext;
|
||||
use source_control as thrift;
|
||||
use source_control::services::source_control_service as service;
|
||||
use srserver::RequestContext;
|
||||
|
||||
use crate::errors;
|
||||
use crate::source_control_impl::SourceControlServiceImpl;
|
||||
|
||||
pub(crate) mod commit;
|
||||
@ -21,10 +21,9 @@ pub(crate) mod tree;
|
||||
impl SourceControlServiceImpl {
|
||||
pub(crate) async fn list_repos(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
_ctx: CoreContext,
|
||||
_params: thrift::ListReposParams,
|
||||
) -> Result<Vec<thrift::Repo>, service::ListReposExn> {
|
||||
let _ctx = self.create_ctx(req_ctxt, None)?;
|
||||
) -> Result<Vec<thrift::Repo>, errors::ServiceError> {
|
||||
let mut repo_names: Vec<_> = self
|
||||
.mononoke
|
||||
.repo_names()
|
||||
|
||||
@ -11,6 +11,7 @@ use std::convert::TryFrom;
|
||||
|
||||
use bytes::Bytes;
|
||||
use chrono::{DateTime, FixedOffset, Local};
|
||||
use context::CoreContext;
|
||||
use futures::stream::Stream;
|
||||
use futures_preview::compat::Future01CompatExt;
|
||||
use futures_util::stream::FuturesOrdered;
|
||||
@ -20,8 +21,6 @@ use mononoke_api::{
|
||||
};
|
||||
use mononoke_types::hash::{Sha1, Sha256};
|
||||
use source_control as thrift;
|
||||
use source_control::services::source_control_service as service;
|
||||
use srserver::RequestContext;
|
||||
|
||||
use crate::commit_id::{map_commit_identities, map_commit_identity, CommitIdExt};
|
||||
use crate::errors;
|
||||
@ -35,11 +34,10 @@ impl SourceControlServiceImpl {
|
||||
/// the requested indentity schemes.
|
||||
pub(crate) async fn repo_resolve_bookmark(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
repo: thrift::RepoSpecifier,
|
||||
params: thrift::RepoResolveBookmarkParams,
|
||||
) -> Result<thrift::RepoResolveBookmarkResponse, service::RepoResolveBookmarkExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&repo))?;
|
||||
) -> Result<thrift::RepoResolveBookmarkResponse, errors::ServiceError> {
|
||||
let repo = self.repo(ctx, &repo)?;
|
||||
match repo.resolve_bookmark(params.bookmark_name).await? {
|
||||
Some(cs) => {
|
||||
@ -59,11 +57,10 @@ impl SourceControlServiceImpl {
|
||||
/// List bookmarks.
|
||||
pub(crate) async fn repo_list_bookmarks(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
repo: thrift::RepoSpecifier,
|
||||
params: thrift::RepoListBookmarksParams,
|
||||
) -> Result<thrift::RepoListBookmarksResponse, service::RepoListBookmarksExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&repo))?;
|
||||
) -> Result<thrift::RepoListBookmarksResponse, errors::ServiceError> {
|
||||
let limit = match check_range_and_convert(
|
||||
"limit",
|
||||
params.limit,
|
||||
@ -98,11 +95,10 @@ impl SourceControlServiceImpl {
|
||||
/// Create a new commit.
|
||||
pub(crate) async fn repo_create_commit(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
repo: thrift::RepoSpecifier,
|
||||
params: thrift::RepoCreateCommitParams,
|
||||
) -> Result<thrift::RepoCreateCommitResponse, service::RepoCreateCommitExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&repo))?;
|
||||
) -> Result<thrift::RepoCreateCommitResponse, errors::ServiceError> {
|
||||
let repo = self.repo(ctx, &repo)?.write().await?;
|
||||
|
||||
let parents: Vec<_> = params
|
||||
|
||||
@ -6,10 +6,10 @@
|
||||
* directory of this source tree.
|
||||
*/
|
||||
|
||||
use context::CoreContext;
|
||||
use source_control as thrift;
|
||||
use source_control::services::source_control_service as service;
|
||||
use srserver::RequestContext;
|
||||
|
||||
use crate::errors;
|
||||
use crate::from_request::check_range_and_convert;
|
||||
use crate::into_response::IntoResponse;
|
||||
use crate::source_control_impl::SourceControlServiceImpl;
|
||||
@ -18,11 +18,10 @@ impl SourceControlServiceImpl {
|
||||
/// List the contents of a directory.
|
||||
pub(crate) async fn tree_list(
|
||||
&self,
|
||||
req_ctxt: &RequestContext,
|
||||
ctx: CoreContext,
|
||||
tree: thrift::TreeSpecifier,
|
||||
params: thrift::TreeListParams,
|
||||
) -> Result<thrift::TreeListResponse, service::TreeListExn> {
|
||||
let ctx = self.create_ctx(req_ctxt, Some(&tree))?;
|
||||
) -> Result<thrift::TreeListResponse, errors::ServiceError> {
|
||||
let (_repo, tree) = self.repo_tree(ctx, &tree).await?;
|
||||
let offset: usize = check_range_and_convert("offset", params.offset, 0..)?;
|
||||
let limit: usize = check_range_and_convert(
|
||||
|
||||
146
scs_server/src/params.rs
Normal file
146
scs_server/src/params.rs
Normal file
@ -0,0 +1,146 @@
|
||||
/*
|
||||
* Copyright (c) Facebook, Inc. and its affiliates.
|
||||
*
|
||||
* This software may be used and distributed according to the terms of the
|
||||
* GNU General Public License found in the LICENSE file in the root
|
||||
* directory of this source tree.
|
||||
*/
|
||||
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
use scuba_ext::{ScubaSampleBuilder, ScubaValue};
|
||||
use source_control as thrift;
|
||||
|
||||
use crate::commit_id::CommitIdExt;
|
||||
|
||||
/// A trait for logging a thrift `Params` struct to scuba.
|
||||
///
|
||||
/// Common and meaningful parameters (e.g. `other_commit` or
|
||||
/// `identity_schemes` are logged using that name. Other parameters
|
||||
/// should have their name prefixed by `param_` to make it clear that
|
||||
/// the column is for a parameter. The default implementation does
|
||||
/// nothing.
|
||||
pub(crate) trait AddScubaParams {
|
||||
fn add_scuba_params(&self, _scuba: &mut ScubaSampleBuilder) {}
|
||||
}
|
||||
|
||||
impl AddScubaParams for BTreeSet<thrift::CommitIdentityScheme> {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add(
|
||||
"identity_schemes",
|
||||
self.iter().map(ToString::to_string).collect::<ScubaValue>(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::ListReposParams {}
|
||||
|
||||
impl AddScubaParams for thrift::RepoCreateCommitParams {}
|
||||
|
||||
impl AddScubaParams for thrift::RepoListBookmarksParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("param_include_scratch", self.include_scratch as i32);
|
||||
scuba.add("param_bookmark_prefix", self.bookmark_prefix.as_str());
|
||||
scuba.add("param_limit", self.limit);
|
||||
self.identity_schemes.add_scuba_params(scuba);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::RepoResolveBookmarkParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("bookmark_name", self.bookmark_name.as_str());
|
||||
self.identity_schemes.add_scuba_params(scuba);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitCompareParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
if let Some(other_commit_id) = self.other_commit_id.as_ref() {
|
||||
scuba.add("other_commit", other_commit_id.to_string());
|
||||
}
|
||||
if let Some(paths) = &self.paths {
|
||||
scuba.add("param_paths", paths.iter().collect::<ScubaValue>());
|
||||
}
|
||||
scuba.add("param_skip_copies_renames", self.skip_copies_renames as i32);
|
||||
self.identity_schemes.add_scuba_params(scuba);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitFileDiffsParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("other_commit", self.other_commit_id.to_string());
|
||||
scuba.add("param_format", self.format.to_string());
|
||||
scuba.add("param_context", self.context);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitFindFilesParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("param_limit", self.limit);
|
||||
if let Some(basenames) = &self.basenames {
|
||||
scuba.add("param_basenames", basenames.iter().collect::<ScubaValue>());
|
||||
}
|
||||
if let Some(prefixes) = &self.prefixes {
|
||||
scuba.add("param_prefixes", prefixes.iter().collect::<ScubaValue>());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitInfoParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
self.identity_schemes.add_scuba_params(scuba);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitIsAncestorOfParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("other_commit", self.other_commit_id.to_string());
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitLookupParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
self.identity_schemes.add_scuba_params(scuba);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitLookupXRepoParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("other_repo", self.other_repo.name.as_str());
|
||||
self.identity_schemes.add_scuba_params(scuba);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitPathBlameParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("param_format", self.format.to_string());
|
||||
// Blame only supports a single identity scheme, but
|
||||
// still log this as a normvector for consistency.
|
||||
scuba.add(
|
||||
"identity_schemes",
|
||||
Some(self.identity_scheme.to_string())
|
||||
.iter()
|
||||
.collect::<ScubaValue>(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::CommitPathInfoParams {}
|
||||
|
||||
impl AddScubaParams for thrift::FileContentChunkParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("param_offset", self.offset);
|
||||
scuba.add("param_size", self.size);
|
||||
}
|
||||
}
|
||||
|
||||
impl AddScubaParams for thrift::FileExistsParams {}
|
||||
|
||||
impl AddScubaParams for thrift::FileInfoParams {}
|
||||
|
||||
impl AddScubaParams for thrift::TreeListParams {
|
||||
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
|
||||
scuba.add("param_offset", self.offset);
|
||||
scuba.add("param_limit", self.limit);
|
||||
}
|
||||
}
|
||||
@ -17,7 +17,7 @@ use mononoke_api::{
|
||||
SessionContainer, TreeContext, TreeId,
|
||||
};
|
||||
use mononoke_types::hash::{Sha1, Sha256};
|
||||
use scuba_ext::{ScubaSampleBuilder, ScubaValue};
|
||||
use scuba_ext::{ScubaSampleBuilder, ScubaSampleBuilderExt, ScubaValue};
|
||||
use slog::Logger;
|
||||
use source_control as thrift;
|
||||
use source_control::server::SourceControlService;
|
||||
@ -28,6 +28,7 @@ use tracing::TraceContext;
|
||||
|
||||
use crate::errors;
|
||||
use crate::from_request::FromRequest;
|
||||
use crate::params::AddScubaParams;
|
||||
use crate::specifiers::SpecifierExt;
|
||||
|
||||
#[derive(Clone)]
|
||||
@ -61,11 +62,15 @@ impl SourceControlServiceImpl {
|
||||
|
||||
pub(crate) fn create_ctx(
|
||||
&self,
|
||||
name: &str,
|
||||
req_ctxt: &RequestContext,
|
||||
specifier: Option<&dyn SpecifierExt>,
|
||||
params: &dyn AddScubaParams,
|
||||
) -> Result<CoreContext, errors::ServiceError> {
|
||||
let mut scuba = self.scuba_builder.clone();
|
||||
scuba.add_common_server_data().add("type", "thrift");
|
||||
scuba.add_common_server_data();
|
||||
scuba.add("type", "thrift");
|
||||
scuba.add("method", name);
|
||||
if let Some(specifier) = specifier {
|
||||
if let Some(reponame) = specifier.scuba_reponame() {
|
||||
scuba.add("reponame", reponame);
|
||||
@ -77,6 +82,7 @@ impl SourceControlServiceImpl {
|
||||
scuba.add("path", path);
|
||||
}
|
||||
}
|
||||
params.add_scuba_params(&mut scuba);
|
||||
let session_id = generate_session_id();
|
||||
scuba.add("session_uuid", session_id.to_string());
|
||||
|
||||
@ -101,7 +107,9 @@ impl SourceControlServiceImpl {
|
||||
None,
|
||||
);
|
||||
|
||||
Ok(session.new_context(self.logger.clone(), scuba))
|
||||
let ctx = session.new_context(self.logger.clone(), scuba);
|
||||
|
||||
Ok(ctx)
|
||||
}
|
||||
|
||||
/// Get the repo specified by a `thrift::RepoSpecifier`.
|
||||
@ -221,24 +229,61 @@ impl SourceControlServiceImpl {
|
||||
}
|
||||
}
|
||||
|
||||
fn log_result<T>(ctx: CoreContext, result: &Result<T, errors::ServiceError>) {
|
||||
let (status, error) = match result {
|
||||
Ok(_) => ("SUCCESS", None),
|
||||
Err(errors::ServiceError::Request(e)) => ("REQUEST_ERROR", Some(format!("{:?}", e))),
|
||||
Err(errors::ServiceError::Internal(e)) => ("INTERNAL_ERROR", Some(format!("{:?}", e))),
|
||||
};
|
||||
let mut scuba = ctx.scuba().clone();
|
||||
scuba.add("status", status);
|
||||
if let Some(error) = error {
|
||||
scuba.add("error", error.as_str());
|
||||
}
|
||||
scuba.log_with_msg("Request complete", None);
|
||||
}
|
||||
|
||||
// Define a macro to construct a CoreContext based on the thrift parameters.
|
||||
macro_rules! create_ctx {
|
||||
( $service_impl:expr, $method_name:ident, $req_ctxt:ident, $params_name:ident ) => {
|
||||
$service_impl.create_ctx(stringify!($method_name), $req_ctxt, None, &$params_name)
|
||||
};
|
||||
|
||||
( $service_impl:expr, $method_name:ident, $req_ctxt:ident, $obj_name:ident, $params_name:ident ) => {
|
||||
$service_impl.create_ctx(
|
||||
stringify!($method_name),
|
||||
$req_ctxt,
|
||||
Some(&$obj_name),
|
||||
&$params_name,
|
||||
)
|
||||
};
|
||||
}
|
||||
|
||||
// Define a macro that generates a non-async wrapper that delegates to the
|
||||
// async implementation of the method.
|
||||
//
|
||||
// The implementations of the methods can be found in the `methods` module.
|
||||
macro_rules! impl_thrift_methods {
|
||||
( $( async fn $method_name:ident($( $param_name:ident : $param_type:ty, )*) -> $result_type:ty; )* ) => {
|
||||
( $( async fn $method_name:ident($( $param_name:ident : $param_type:ty, )*) -> Result<$ok_type:ty, $err_type:ty>; )* ) => {
|
||||
$(
|
||||
fn $method_name<'implementation, 'req_ctxt, 'async_trait>(
|
||||
&'implementation self,
|
||||
req_ctxt: &'req_ctxt RequestContext,
|
||||
$( $param_name: $param_type ),*
|
||||
) -> Pin<Box<dyn Future<Output = $result_type> + Send + 'async_trait>>
|
||||
) -> Pin<Box<dyn Future<Output = Result<$ok_type, $err_type>> + Send + 'async_trait>>
|
||||
where
|
||||
'implementation: 'async_trait,
|
||||
'req_ctxt: 'async_trait,
|
||||
Self: Sync + 'async_trait,
|
||||
{
|
||||
Box::pin((self.0).$method_name(req_ctxt, $( $param_name ),* ))
|
||||
let handler = async move {
|
||||
let ctx = create_ctx!(self.0, $method_name, req_ctxt, $( $param_name ),*)?;
|
||||
ctx.scuba().clone().log_with_msg("Request start", None);
|
||||
let res = (self.0).$method_name(ctx.clone(), $( $param_name ),* ).await;
|
||||
log_result(ctx, &res);
|
||||
Ok(res?)
|
||||
};
|
||||
Box::pin(handler)
|
||||
}
|
||||
)*
|
||||
}
|
||||
|
||||
@ -1405,3 +1405,16 @@ function git() {
|
||||
GIT_AUTHOR_EMAIL="$email" \
|
||||
command git "$@"
|
||||
}
|
||||
|
||||
function summarize_scuba_json() {
|
||||
local interesting_tags
|
||||
local key_spec
|
||||
interesting_tags="$1"
|
||||
shift
|
||||
key_spec=""
|
||||
for key in "$@"
|
||||
do
|
||||
key_spec="$key_spec + (if (${key} != null) then {${key##*.}: ${key}} else {} end)"
|
||||
done
|
||||
jq -S "if (.normal.log_tag | match(\"^($interesting_tags)\$\")) then ${key_spec:3} else empty end"
|
||||
}
|
||||
|
||||
@ -67,10 +67,111 @@ try talking to the server before it is up
|
||||
start SCS server
|
||||
$ start_and_wait_for_scs_server --scuba-log-file "$TESTTMP/scuba.json"
|
||||
|
||||
make some simple requests that we can use to check scuba logging
|
||||
|
||||
repos
|
||||
$ scsc repos
|
||||
repo
|
||||
|
||||
lookup using bookmark
|
||||
$ scsc lookup --repo repo -B BOOKMARK_C -S bonsai
|
||||
006c988c4a9f60080a6bc2a2fff47565fafea2ca5b16c4d994aecdef0c89973b
|
||||
|
||||
diff paths only
|
||||
$ scsc diff --repo repo --paths-only -B BOOKMARK_B --bonsai-id "006c988c4a9f60080a6bc2a2fff47565fafea2ca5b16c4d994aecdef0c89973b"
|
||||
M b
|
||||
A binary
|
||||
A c
|
||||
|
||||
check the scuba logs
|
||||
$ summarize_scuba_json "Request.*" < "$TESTTMP/scuba.json" \
|
||||
> .normal.log_tag .normal.msg .normal.method \
|
||||
> .normal.commit .normal.other_commit .normal.path \
|
||||
> .normal.bookmark_name .normvector.identity_schemes \
|
||||
> .normal.status .normal.error
|
||||
{
|
||||
"log_tag": "Request start",
|
||||
"method": "list_repos"
|
||||
}
|
||||
{
|
||||
"log_tag": "Request complete",
|
||||
"method": "list_repos",
|
||||
"status": "SUCCESS"
|
||||
}
|
||||
{
|
||||
"bookmark_name": "BOOKMARK_C",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request start",
|
||||
"method": "repo_resolve_bookmark"
|
||||
}
|
||||
{
|
||||
"bookmark_name": "BOOKMARK_C",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request complete",
|
||||
"method": "repo_resolve_bookmark",
|
||||
"status": "SUCCESS"
|
||||
}
|
||||
{
|
||||
"commit": "006c988c4a9f60080a6bc2a2fff47565fafea2ca5b16c4d994aecdef0c89973b",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request start",
|
||||
"method": "commit_lookup"
|
||||
}
|
||||
{
|
||||
"commit": "006c988c4a9f60080a6bc2a2fff47565fafea2ca5b16c4d994aecdef0c89973b",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request complete",
|
||||
"method": "commit_lookup",
|
||||
"status": "SUCCESS"
|
||||
}
|
||||
{
|
||||
"bookmark_name": "BOOKMARK_B",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request start",
|
||||
"method": "repo_resolve_bookmark"
|
||||
}
|
||||
{
|
||||
"bookmark_name": "BOOKMARK_B",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request complete",
|
||||
"method": "repo_resolve_bookmark",
|
||||
"status": "SUCCESS"
|
||||
}
|
||||
{
|
||||
"commit": "006c988c4a9f60080a6bc2a2fff47565fafea2ca5b16c4d994aecdef0c89973b",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request start",
|
||||
"method": "commit_compare",
|
||||
"other_commit": "c63b71178d240f05632379cf7345e139fe5d4eb1deca50b3e23c26115493bbbb"
|
||||
}
|
||||
{
|
||||
"commit": "006c988c4a9f60080a6bc2a2fff47565fafea2ca5b16c4d994aecdef0c89973b",
|
||||
"identity_schemes": [
|
||||
"BONSAI"
|
||||
],
|
||||
"log_tag": "Request complete",
|
||||
"method": "commit_compare",
|
||||
"other_commit": "c63b71178d240f05632379cf7345e139fe5d4eb1deca50b3e23c26115493bbbb",
|
||||
"status": "SUCCESS"
|
||||
}
|
||||
|
||||
commands after this point may run requests in parallel, which can change the ordering
|
||||
of the scuba samples.
|
||||
|
||||
diff
|
||||
$ scsc diff --repo repo -B BOOKMARK_B -i "$COMMIT_C"
|
||||
diff --git a/b b/b
|
||||
@ -238,10 +339,6 @@ blame
|
||||
}
|
||||
]
|
||||
|
||||
lookup using bookmarks
|
||||
$ scsc lookup --repo repo -B BOOKMARK_B
|
||||
323afe77a1b1e632e54e8d5a683ba2cc8511f299
|
||||
|
||||
lookup, commit without globalrev
|
||||
$ scsc lookup --repo repo -B BOOKMARK_B -S bonsai,hg,globalrev
|
||||
bonsai=c63b71178d240f05632379cf7345e139fe5d4eb1deca50b3e23c26115493bbbb
|
||||
@ -354,7 +451,3 @@ list directory
|
||||
file 10 b
|
||||
file 5 binary
|
||||
file 0 c
|
||||
|
||||
check scuba logs
|
||||
$ interesting_tags="TBD"
|
||||
$ jq "if (.normal.log_tag | match(\"^($interesting_tags)\$\")) then (.normal.log_tag, .normal.msg) else empty end" < "$TESTTMP/scuba.json"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user