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

Some small instrumenting improvements #18

Merged
merged 4 commits into from
Jul 25, 2023
Merged

Conversation

Hoverbear
Copy link
Contributor

Just some small instrumenting improvements, nothing life changing here.

@Hoverbear Hoverbear self-assigned this Jul 21, 2023

tracing::trace!(
endpoint = %crate::graphql::GITHUB_ENDPOINT,
query = serde_json::to_string(&query).unwrap_or_else(|_| "<error serializing>".into()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#17 makes the query implement Debug so we probably don't need this extra serialisation stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the proc macro created Variables type is the problem:

   Compiling nxfr-push v0.1.0 (/home/ana/git/determinatesystems/nxfr-push)
error[E0277]: `Variables` doesn't implement `std::fmt::Debug`
   --> src/release_metadata.rs:141:9
    |
141 | /         tracing::trace!(
142 | |             endpoint = %crate::graphql::GITHUB_ENDPOINT,
143 | |             ?query,
144 | |             "Making request"
145 | |         );
    | |         ^
    | |         |
    | |_________`Variables` cannot be formatted using `{:?}`
    |           required by a bound introduced by this call
    |
    = help: the trait `std::fmt::Debug` is not implemented for `Variables`
    = note: add `#[derive(Debug)]` to `Variables` or manually `impl std::fmt::Debug for Variables`
    = help: the trait `std::fmt::Debug` is implemented for `QueryBody<Variables>`
    = note: required for `QueryBody<Variables>` to implement `std::fmt::Debug`
    = note: 1 redundant requirement hidden
    = note: required for `&QueryBody<Variables>` to implement `std::fmt::Debug`
note: required by a bound in `tracing::field::debug`
   --> /home/ana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-core-0.1.31/src/field.rs:365:8
    |
363 | pub fn debug<T>(t: T) -> DebugValue<T>
    |        ----- required by a bound in this function
364 | where
365 |     T: fmt::Debug,
    |        ^^^^^^^^^^ required by this bound in `debug`
    = note: this error originates in the macro `$crate::valueset` which comes from the expansion of the macro `tracing::trace` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `Variables` with `#[derive(Debug)]`
   --> src/graphql/mod.rs:7:10
    |
7   | #[derive(#[derive(Debug)]
    |          ++++++++++++++++

However your question prompted me to dig around and find out that we can set variables_derives:

#[derive(GraphQLQuery)]
#[graphql(
    schema_path = "src/graphql/github_schema.graphql",
    query_path = "src/graphql/rev_count_query.graphql",
    response_derives = "Debug",
    variables_derives = "Debug",
)]
pub(crate) struct RevCountQuery;

@grahamc grahamc merged commit 428371d into main Jul 25, 2023
5 checks passed
@grahamc grahamc deleted the instrumenting-improvements branch July 25, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants