-
Notifications
You must be signed in to change notification settings - Fork 1
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
STR-440 OpenTelemetry tracing exporter #342
Conversation
This does not compile because of conflicting versions of Tokio and the various OTLP crates. Opening this so others can look at it and maybe pick it up. |
a462eae
to
8d47140
Compare
This deletes Cargo.lock because we couldn't get the deps to work anyways.
now it compiles at least-ish
8d47140
to
973f1fc
Compare
Alright so it builds and inits properly, but now we're getting this issue when we try to run the client in a local test env with a url to a local Jaeger instance running:
We can run Jaeger now with |
Alright I tried it with opentelemetry-collector and it seems like Jaeger was just not listening for some inexplicable reason. This doesn't give the nice UI but it does show that the trace events are being generating by spamming strata-looking strings out in the docker output. I will investigate further but it's good to know it seems like this is merely a configuration issue now. |
Figured out the Jaeger thing, I was just using an ancient version of Jaeger because I copypasted a docker command from their docs that wasn't recent, so it didn't support the OpenTelemetry collector. |
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #342 +/- ##
==========================================
+ Coverage 57.06% 57.19% +0.12%
==========================================
Files 255 257 +2
Lines 26993 27226 +233
==========================================
+ Hits 15403 15571 +168
- Misses 11590 11655 +65
|
Should figure out a better solution to the logging in tests, maybe should make a simpler stub. |
logging::init(); | ||
// FIXME this is absurd, why are we doing this here? | ||
logging::init(logging::LoggerConfig::with_base_name("strata-client-tests")); |
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.
It this calling btcio::client
by any means?
If yes there's a lot of good logging happening there that is quite useful.
* common: tried to set up otlp This deletes Cargo.lock because we couldn't get the deps to work anyways. * fix: bump opentelemetry stuff to 0.26 now it compiles at least-ish * common: added more correct init for OTLP tracing output * common, contrib: fixed some more logging stuff, added Jaeger util script * common, stratad: reworked logging init some more to try to make it work * contrib: fixed Jaeger init script to use a recent version * bridge-client: fixed broken test, fixed Tokio version * common: added `service.name` property to OTLP params * prover-client: fixed missing `LoggerConfig` * stratad: fixed broken tests??? idk why this is here * tests: fixed broken logging * btcio: fixed broken logging --------- Co-authored-by: Jose Storopoli <[email protected]>
STR-440 OpenTelemetry tracing exporter (#342) * common: tried to set up otlp This deletes Cargo.lock because we couldn't get the deps to work anyways. * fix: bump opentelemetry stuff to 0.26 now it compiles at least-ish * common: added more correct init for OTLP tracing output * common, contrib: fixed some more logging stuff, added Jaeger util script * common, stratad: reworked logging init some more to try to make it work * contrib: fixed Jaeger init script to use a recent version * bridge-client: fixed broken test, fixed Tokio version * common: added `service.name` property to OTLP params * prover-client: fixed missing `LoggerConfig` * stratad: fixed broken tests??? idk why this is here * tests: fixed broken logging * btcio: fixed broken logging --------- Co-authored-by: Trey Del Bonis <[email protected]>
Description
This PR is to support the OpenTelemetry exporter so we can have richer logging from our production environment.
Type of Change
Checklist