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

Tracking issue for USDT Integration #298

Closed
6 tasks
smklein opened this issue Oct 8, 2021 · 12 comments
Closed
6 tasks

Tracking issue for USDT Integration #298

smklein opened this issue Oct 8, 2021 · 12 comments
Labels
Debugging For when you want better data in debugging an issue (log messages, post mortem debugging, and more) enhancement New feature or request.

Comments

@smklein
Copy link
Collaborator

smklein commented Oct 8, 2021

https://crates.io/crates/usdt is super sick, and it's about time we start integrating! This issue tracks integration into a variety of potential spots within omicron:

  • Sled Agent (device management)
  • Sled Agent (interfacing w/Propolis)
  • Sled Agent's bootstrapping
  • Nexus (APIs)
  • Nexus (DB) interface
  • ... other?
@smklein smklein added enhancement New feature or request. 👋 good first issue labels Oct 8, 2021
@bnaecker
Copy link
Collaborator

bnaecker commented Oct 8, 2021

Just a thought. We'd talked previously about getting the actual SQL strings at various points -- this might be a good candidate. We might not want to log them all the time, but a probe spitting it out could be very helpful.

@davepacheco
Copy link
Collaborator

I think we could get pretty far with:

  • dropshot per-request probes
  • slog probes for every message logged (even ones being filtered out)
  • probes for database operations (is there a central place we can put this?)

@bnaecker
Copy link
Collaborator

bnaecker commented Oct 8, 2021

@davepacheco USDT requires nightly, and there's no easy way to instruct it to generate empty probes if nightly isn't opted into. Are you cool with transferring that requirement to Dropshot?

@smklein
Copy link
Collaborator Author

smklein commented Oct 8, 2021

async-bb8-diesel implements batch_execute_async with a query string - we could fire a probe for each one within that crate?

https://github.com/oxidecomputer/async-bb8-diesel/blob/dc5ff98304efe98dfb9736a4634e5841108690bb/src/connection_manager.rs#L115-L123

(We could also do the same thing for the per-connection implementation: https://github.com/oxidecomputer/async-bb8-diesel/blob/dc5ff98304efe98dfb9736a4634e5841108690bb/src/connection.rs#L39-L46 )

This would effectively look like:

    async fn batch_execute_async(&self, query: &str) -> ConnectionResult<()> {
        let diesel_conn = Connection(self.0.clone());
        let query = query.to_string();
        task::spawn_blocking(move || {
               /* ++ FIRE PROBE HERE, RECORD QUERY ++ */
               diesel_conn.inner().batch_execute(&query)
            })
            .await
            .unwrap() // Propagate panics
            .map_err(ConnectionError::from)
    }

@davepacheco
Copy link
Collaborator

@bnaecker What about adding a Dropshot (or usdt) feature such that:

  • if the feature is enabled, you get probes and must be on nightly
  • if the feature is not enabled, then you don't get probes and can use whatever toolchain you want

It's not as slick as automatically detecting this but it might even be better. It avoids a problem I've run into in the past where I thought something should have DTrace probes, but a bug in the build process failed to detect support, and we didn't discover that until we needed the probes in production.

@davepacheco
Copy link
Collaborator

@smklein That sounds good.

@bnaecker
Copy link
Collaborator

bnaecker commented Oct 8, 2021

@davepacheco That should be fairly easy to add into USDT. We already have a no-op implementation for platforms without DTrace, so adding a feature that allows specifically selecting it would be straightforward. Dropshot could then re-export that feature.

@bnaecker
Copy link
Collaborator

Just an update on some of the upstream work. Probes for log messages can be inserted via the slog-dtrace crate. Probes for Dropshot requests are part of this PR. Probes for database operations (via async-bb8-diesel) are part of this PR. More nuanced probes around database operations would be a bit more work, but this should allow to us to instrument every query.

@bnaecker
Copy link
Collaborator

bnaecker commented Dec 7, 2021

The database-related probes, for connections and queries, has been implemented as part of #488.

@bnaecker
Copy link
Collaborator

bnaecker commented Dec 8, 2021

Log messages are emitted to DTrace as part of #489.

@jordanhendricks jordanhendricks added the Debugging For when you want better data in debugging an issue (log messages, post mortem debugging, and more) label Aug 11, 2023
@jordanhendricks
Copy link
Contributor

I have been looking through issues to find things related to development and debugging. Can this one be closed?

@bnaecker
Copy link
Collaborator

I think we've gotten a good core of probes, and hopefully folks find it easy enough to add more. I'll close this now, thanks @jordanhendricks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Debugging For when you want better data in debugging an issue (log messages, post mortem debugging, and more) enhancement New feature or request.
Projects
None yet
Development

No branches or pull requests

5 participants