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

Add OpenTelemetry to Rust benchmark runner #75

Merged
merged 7 commits into from
Sep 25, 2024
Merged

Add OpenTelemetry to Rust benchmark runner #75

merged 7 commits into from
Sep 25, 2024

Conversation

graebm
Copy link
Contributor

@graebm graebm commented Sep 24, 2024

Get started adding observability, so we can diagnose bottlenecks.

Many crates (including the AWS SDK for Rust) use the tracing crate to emit Spans and Events. We want to view this data.

OpenTelemetry is a widely use ecosystem for viewing this kind of data, so let's use the opentelemetry-* crates (various features are split among various crates).

OTLP/gRPC (OpenTelemetry Protocol) is a popular and platform-agnostic way to export this data, so let's use the opentelemetry-otlp crate.

The tracing-opentelemetry crate exists to help pipe data from tracing to an opentelemetry::trace::Tracer, so let's use that.

There's lots more to do. Currently, it's hard-coded to only view "info" level data because "debug" was too much. I'm still learning how to filter for the exact data we're interested in.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Using older version of tracing_openetelemetry (v0.24.0 and its dependencies), because the latest version seems to be broken (its otlp example doesn't export data correctly)
@landonxjames
Copy link

HIgh level request: Could we get a comment outlining all of the troubleshooting and exact version matching you had to do to get this working? I know it involved a lot of banging your head against various walls, and there are some indications of it like the comment in Cargo.toml, but it would be nice to have it all laid out in detail somewhere so the next person to touch this gets some amount of context from the start.

@graebm
Copy link
Contributor Author

graebm commented Sep 25, 2024

HIgh level request: Could we get a comment outlining all of the troubleshooting and exact version matching you had to do to get this working? I know it involved a lot of banging your head against various walls, and there are some indications of it like the comment in Cargo.toml, but it would be nice to have it all laid out in detail somewhere so the next person to touch this gets some amount of context from the start.

I'll add a comment in telemetry.rs, but I'm still just going to link the issue: tokio-rs/tracing-opentelemetry#159 since it has all the context, and repro steps, and will eventually have the version it's fixed in.

A lot of the head-banging was specific to me learning about this space. Such as:

  • Wondering why, when running the basic-otlp example from the opentelemetry-rust repo, I could only see SOME spans and events. Like I could see this event, but not this one, and not anything from my benchmark code, or from the Rust SDK.
    • Answer: this sample never sets up a tracing_subscriber, so you'll only see spans/events emitted via the opentelemetry APIs. You won't see spans/events emitted via the tracing API.
    • The tracing-opentelemetry can bridge these crates, but it's owned by the Tokio folks, which I'm guessing is why the examples in the OpenTelemetry repo aren't showing how to use it? I saw some comment that OpenTelemetry is going to make their own crate for this, so hopefully it's easier in the future.
  • Once I understood the concept of a tracing_subscriber I found the tracing-opentelemetry crate with its own opentelemetry-otlp example, but couldn't figure out why I still couldn't see any spans/events.
    • Answer: this example is currently broken, the latest versions of all these libraries don't work together at the moment
  • I saw a comment somewhere about using an older version and kinda got that working, but when I started mixing in any real code it would stop exporting anything.
    • Answer: At this point, do to sometimes inconsistent usage of major.minor vs major.minor.patch in my Cargo.toml's dependencies, i was pulling in multiple different versions of the same opentelemetry crate, which I didn't even realize was possible. Once I got the exact 1 version of these dependencies it finally started working

Copy link

@aajtodd aajtodd left a comment

Choose a reason for hiding this comment

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

Nothing blocking.

runners/s3-benchrunner-rust/src/transfer_manager.rs Outdated Show resolved Hide resolved
runners/s3-benchrunner-rust/Cargo.toml Outdated Show resolved Hide resolved
@graebm graebm enabled auto-merge (squash) September 25, 2024 17:55
@graebm graebm merged commit ba1aa4d into main Sep 25, 2024
5 checks passed
@graebm graebm deleted the rust-otel3 branch September 25, 2024 17:58
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.

4 participants