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

Features for v3 #275

Open
Diggsey opened this issue Oct 30, 2020 · 6 comments
Open

Features for v3 #275

Diggsey opened this issue Oct 30, 2020 · 6 comments

Comments

@Diggsey
Copy link

Diggsey commented Oct 30, 2020

Problems

Mutable scopes

Typically applications are structured such that there is some scope (eg. a request, a transaction, etc.) in which we want to add some log keys.

However, it is not always the case that these keys are known at the start of the scope (for example, once we have authenticated a request, we want to include the user ID in further logs, but we don't know that at the outset). To solve this, adding keys should be decoupled from beginning a scope.

Boilerplate / interoperability

The "scope" information is not only useful for slog: that information should also be accessible to an applications error-reporting solution, to telemetry systems (eg. statsd) and other tracing systems.

At the moment, this involves duplicating a lot of information as each system wants to own the way these scopes are managed. Pulling keys back out from slog is very painful.

Evaluated keys

The PushFnValue and FnValue types are very useful, but they are not particularly efficient: lets say I have a scoped-thread-local called TracingCtx with a bunch of fields, and I want to add these fields to log entries. I have to have a separate PushFnValue for each field, meaning I have to access the thread-local multiple times per log entry. It would be much more efficient if I could access the thread-local once and emit a sequence of fields.

Solution

Instead of converting everything to key-value pairs up-front, integrate with serde. Usage would look something like this:

#[derive(Serialize, Debug)]
struct MyRequestCtx {
    url: String,
    method: Method,
    user_id: Option<Uuid>,
}

// Using `scoped_tls_hkt` which allows defining mutable scoped thread locals
scoped_thread_local!(static mut REQUEST_CTX: MyRequestCtx);

fn main() {
    ...
    // This supercedes `slog_scope`.
    // Whenever something is logged, the logger will check each context. If that context is currently set,
    // it will be serialized to with serde, and all keys will be prefixed with "request."
    root_logger.add_context("request.", &REQUEST_CTX);
}

fn handle_request(req: &Request) -> Response {
    // This creates the request scope.
    // Within `inner_handle_request`, all logs will automatically have the "request.url" and "request.method"
    // keys attached.
    REQUEST_CTX.set(&mut MyRequestCtx {
        url: req.url(),
        method: req.method(),
        user_id: None,
    }, inner_handle_request)
}

fn inner_handle_request(req: &Request) -> Response {
    ...
    if let Some(user_id) = authenticate(req) {
        // A feature of `scoped-tls-hkt` is that we can mutate the thread-local to add
        // additional context later on:
        REQUEST_CTX.with(|ctx| ctx.user_id = Some(user_id));
        // This information will persist for the remainder of the request.
    }
    ...
}
@Kixunil
Copy link
Contributor

Kixunil commented Jan 12, 2021

Is there anything wrong with:

let mut logger = ...;
// ...
logger = logger.new(o!());

?

@dpc
Copy link
Collaborator

dpc commented Jan 12, 2021

Seems to me like this should be implementable as a 3rd party library.

@Diggsey
Copy link
Author

Diggsey commented Jan 12, 2021

@Kixunil I'm afraid I don't see how that addresses the same issue.

@dpc Possibly it could be implemented as a whole-sale replacement of slog-scope...

@dpc
Copy link
Collaborator

dpc commented Jan 12, 2021

slog-scope is already a 3rd party library, yes.

@Diggsey
Copy link
Author

Diggsey commented Jan 12, 2021

It doesn't have a separate issue tracker though, so this seemed the right place to discuss.

@Kixunil
Copy link
Contributor

Kixunil commented Jan 12, 2021

Ah, this is about slog-scope, I thought you wanted to change the keys inside logger. Sorry about confusion.

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

No branches or pull requests

3 participants