-
Notifications
You must be signed in to change notification settings - Fork 0
feat(macro): add macros to trace error including stack, trace and backtrace #154
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
base: master
Are you sure you want to change the base?
Conversation
cpiemontese
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, not sure what that the intermediaries code contains and how intermediaries-specific it is, but if it can enrich prima_tracing.rs why not
I'm trying to decide whether to implement everything here or use an existing crate that does the same thing, like this one. |
|
Using something existing sounds very enticing tbh 😁 Not sure how the features compare with the custom lib |
25fdd5a to
e3ff4a5
Compare
e3ff4a5 to
5815ce1
Compare
5815ce1 to
7808a50
Compare
|
I see the CI is still red, will #157 fix it? |
7808a50 to
5b87037
Compare
Yeah, I just rebase from master |
cpiemontese
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments, mostly related to typos
In general, looking at the otel semconvs it seems like this is fine, most of these fields are custom to us (e.g. stack is not part of the error semconv)
Also, I don't know how much Datadog care about this semconvs
For the actual macros, I will give time to someone else to review since I'm not really familiar with them
| error-report = "0.0.1" | ||
|
|
||
| [dev-dependencies] | ||
| actix-web = "4.0.1" | ||
| opentelemetry-jaeger = {version = "0.22", features = ["integration_test"]} | ||
| prima_bridge = "0.25" | ||
| tokio = {version = "1.17", features = ["rt", "macros", "rt-multi-thread"]} | ||
| tracing-actix-web = {version = "0.7.11", features = ["opentelemetry_0_27"]} | ||
| tracing-capture = "0.1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't understand what these definitions import, are these internal crates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to explain them.
error-report -> https://docs.rs/error-report/0.0.1/error_report
Report prints an error and all its sources.
Source messages will be cleaned using CleanedErrors to remove duplication from errors that include their source’s message in their own message.
The debug implementation prints each error on a separate line, while the display implementation prints all errors on one line separated by a colon. Using alternate formatting ({:#}) is identical to the debug implementation.
tracing-capture -> https://docs.rs/tracing-capture/0.1.0/tracing_capture/
This crate provides a tracing Layer to capture tracing spans and events as they occur.
The captured spans and events can then be used for testing assertions (e.g., "Did a span with a specific name / target / … occur? What were its fields? Was the span closed? How many times the span was entered?" and so on).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Word of advise @damjack , the error-report crate used in starsky and incentives-backend is not https://docs.rs/error-report, but https://github.com/primait/intermediaries-crates/tree/master/error-report
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reference
I see tracing_capture is just imported in tests, is it just a local dev library?
For error-report the fact that
- it is 0.0.1
- has no associated repo
kinda scares me off of depending on it, could the code be ported directly here...?
Not sure if we need everything contained in the source, I get wanting to "pretty print" errors but we already depend on shaky libs as is
Co-authored-by: Cristiano Piemontese <[email protected]>
Co-authored-by: Cristiano Piemontese <[email protected]>
Hi, I would like to propose to add macros to make it easy to record and emit errors to tracing spans and events while including structured error data, the error chain (source), and a stack/backtrace.
The new macros reduce boilerplate when converting Result/Errors into tracing events, attaching contextual fields (error, cause chain, span/trace ids), and capturing a Backtrace when the feature is enabled.
So in short what I would like to have is:
One plus I added is to handle also
anyhowerror crate:Motivation
Tests & docs