Skip to content

RSCBC-12: Implement tracing #245

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

Open
wants to merge 1 commit into
base: nativex
Choose a base branch
from

Conversation

Matt-Woz
Copy link

No description provided.

@Matt-Woz Matt-Woz force-pushed the tracing_and_metrics branch 3 times, most recently from 2e65ddb to 008c77c Compare February 26, 2025 21:58
@Matt-Woz Matt-Woz force-pushed the tracing_and_metrics branch from 008c77c to 58a4e69 Compare February 26, 2025 22:09
if !log_output.is_empty() {
match serde_json::to_string(&log_output) {
Ok(log_output_str) => {
tracing::warn!("Operations over threshold: {}", log_output_str)
Copy link
Author

@Matt-Woz Matt-Woz Feb 27, 2025

Choose a reason for hiding this comment

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

I think given we're using tracing for spans already, it probably makes sense to convert all of our logging to use tracing instead, since tracing can be configured to also emit log records. See the tracing docs on log crate compatibility

@@ -462,6 +489,7 @@ impl Agent {
info!("Orphan : {:?}", packet);
}),
disable_decompression: opts.compression_config.disable_decompression,
tracing: tracing.clone(),
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure this is the best way to get the tracing component down the KV stack to the KvClient, but I can't see a nicer way -- Given spans can be manipulated w/ static functions, I did also experiment with just passing the TracingConfig (i.e. ClusterLabels) in with the KvConfig, but I don't think it was much better

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.

1 participant