-
Notifications
You must be signed in to change notification settings - Fork 26
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
Inital opentelemetry
layout
#400
base: main
Are you sure you want to change the base?
Conversation
opentelemetry-http = { version = "0.27.0", default-features = false} | ||
opentelemetry-otlp = { version = "0.27.0", features = ["logs", "http-json", "reqwest-client"] } |
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.
note: I used the http+json
version of opentelemetry instead of the grpc+protobuf
I did this because we currently only support the http+json
approach in Paima at the moment, but plan to support grpc+protobuf in the future
@@ -62,6 +69,19 @@ pub fn open_data_stores(config: &crate::Config) -> Result<Stores, Error> { | |||
pub fn setup_tracing(config: &LoggingConfig) -> miette::Result<()> { | |||
let level = config.max_level; | |||
|
|||
let exporter = LogExporter::builder() | |||
.with_http() | |||
.with_endpoint("http://localhost:4318/v1/logs") |
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.
note: this should be configured automatically for you from an env variable (https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#endpoint-urls-for-otlphttp), but I just set it explicitly here
#[tokio::main] | ||
async fn main() -> Result<()> { |
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.
If I don't have this, cargo run sync
gives me the error
thread 'main' panicked at /home/sebdev20/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-util-0.1.5/src/client/legacy/connect/dns.rs:121:24:
there is no reactor running, must be called from the context of a Tokio 1.x runtime
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Note that I don't need this to run cargo run daemon
- that one instead starts but then deadlocks right away. I tried switching to with_batch_exporter
which doesn't deadlock, but instead of doesn't seem to actually send anything to opentelemetry either (not sure why)
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 realize now that unfortunately this breaks cargo run daemon
with the following error
thread 'main' panicked at /home/me/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.40.0/src/runtime/scheduler/multi_thread/mod.rs:86:9:
Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.
note: cargo run sync
still works
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.
They recently (December 2024) updated the docs on how to get the SimpleExporter
working to avoid the deadlock mentioned earlier
However, it turns out that, unfortunately, using the SimpleExporter
requires a fairly different setup than the batch exporter:
SimpleExporter
requires enabling feature flagreqwest-blocking-client
and making themain()
a normal main and nottokio::main
However, I can't seems to get it to work because
reqwest-blocking-client
as recommended errors with "Cannot drop a runtime in a context where blocking is not allowed" panic in the blocking Client seanmonstar/reqwest#1017- using
reqwest-client
givesthere is no reactor running, must be called from the context of a Tokio 1.x runtime
(and adding this runs into the issue withcargo run daemon
mentioned above)
There are some issues that were resolved in opentelemetry that sound like they might be relevant such as open-telemetry/opentelemetry-rust#2431, but they are not released yet. More generally, I wonder if open-telemetry/opentelemetry-rust#2386 might make the integration easier
.build().unwrap(); | ||
|
||
let provider = LoggerProvider::builder() | ||
.with_simple_exporter(exporter) |
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.
we want to use with_simple_exporter
since it's a lot more responsive in dev mode (and for Paima you're running everything on the same machine so the network overhead is minimal), but for production where you have different machines with_batch_exporter
will work a lot better
@@ -9,6 +9,13 @@ use tokio::task::JoinHandle; | |||
use tokio_util::sync::CancellationToken; | |||
use tracing::{debug, warn}; | |||
use tracing_subscriber::{filter::Targets, prelude::*}; | |||
use opentelemetry_appender_tracing::layer; | |||
use opentelemetry_otlp::WithExportConfig; | |||
use opentelemetry_otlp::{LogExporter, Protocol}; |
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.
Note: I only export logs and not traces (spans) or metrics just because I only needed logs to make sure it works
@@ -32,6 +32,11 @@ thiserror = "1.0.30" | |||
lazy_static = "1.4.0" | |||
tracing = "0.1.37" | |||
tracing-subscriber = "0.3.17" | |||
opentelemetry-appender-tracing = { version = "0.27.0", default-features = false} |
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.
note: I only added the system to forward the tracing
crate to opentelemtry, but not other crates
that is to say, I noticed in Dolos (and maybe downstream crates?) you occasionally use use log::
which won't be forwarded
I added code to get opentelemtry working so I could consume it from Paima Engine
Closes #399