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

feat: stack trace style debug print for error #2489

Merged
merged 10 commits into from
Sep 26, 2023

Conversation

waynexia
Copy link
Member

@waynexia waynexia commented Sep 25, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

TL;DR

Change the error logging style to

(by error! macro)

2023-09-25T16:56:56.012879Z ERROR frontend::instance: Failed to execute query: select count(*) from tt trace_id=7112117781994278907 err.msg=0: Table operation error, at src/frontend/src/instance.rs:417:14
1: Failed to plan statement
2: Cannot plan SQL: SELECT count(*) FROM tt, at src/query/src/planner.rs:79:60
3: Plan("Table not found: greptime.public.tt") err.code=PlanQuery

(by debug print)

0: Table operation error, at src/frontend/src/instance.rs:417:14
1: Failed to plan statement
2: Cannot plan SQL: SELECT count(*) FROM tt, at src/query/src/planner.rs:79:60
3: Plan("Table not found: greptime.public.tt")

And the previous one looks like

2023-09-25T16:37:56.266934Z ERROR frontend::instance: Failed to execute query: select count(*) from tt trace_id=7112112991553851383 err.msg=Table operation error err.code=PlanQuery err.source=Failed to plan statement err.source.sources=[Cannot plan SQL: SELECT count(*) FROM tt, Error during planning: Table not found: greptime.public.tt]

How to use

Take cmd/error.rs as an example:

-#[derive(Snafu)]
#[snafu(visibility(pub))]
+#[stack_trace_debug]
pub enum Error {
    #[snafu(display("Failed to create default catalog and schema"))]
    InitMetadata {
        location: Location,
        source: common_meta::error::Error,
    },

    #[snafu(display("Cannot create REPL"))]
    ReplCreation {
+        #[snafu(source)]
+        error: ReadlineError,
        location: Location,
    },
}

Only a few things to notice:

  • need not derive Debug manually, the new macro stack_trace_debug will generate one that can print the error prettier
  • add #[stack_trace_debug] to every error enum
  • For external errors, change its name to error and add a snafu annotation #[snafu(source)].

todo

  • remove location_opt
  • maybe add a method to retrieve external error source

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #2489 (217e6e6) into develop (230a302) will decrease coverage by 0.39%.
The diff coverage is 78.54%.

❗ Current head 217e6e6 differs from pull request most recent head a8a8b85. Consider uploading reports for the commit a8a8b85 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2489      +/-   ##
===========================================
- Coverage    85.02%   84.64%   -0.39%     
===========================================
  Files          724      725       +1     
  Lines       115337   115542     +205     
===========================================
- Hits         98071    97800     -271     
- Misses       17266    17742     +476     

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@shuiyisong shuiyisong self-requested a review September 26, 2023 07:10
Copy link
Contributor

@shuiyisong shuiyisong left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

Since this PR is easy to conflict with others, I decided to merge it first.

We could implement the enhancement mentioned in #2489 (comment) later

@waynexia waynexia added this pull request to the merge queue Sep 26, 2023
Merged via the queue into GreptimeTeam:develop with commit 515ce82 Sep 26, 2023
12 checks passed
@waynexia waynexia deleted the stack-trace-debug branch September 26, 2023 11:36
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* impl macro stack_trace_debug

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* manually mark external error

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* ignore warnings

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* fix clippy warnings

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* use debug print

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* simplify the error and warn macro

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* fix ut

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* add docs

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* replace snafu backtrace with location

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

---------

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
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