Leverage static str key when possible #727

2026-05-31T23:27:58-05:00

tisonkun commented

2026-05-31 23:27:58 -05:00

(Migrated from github.com)

Replace &'k str in Key<'k> with MaybeStaticStr<'k> to leverage &'static str when in most cases it is.

Open for review, especially on:

Whether it is backward compatible.
Whether MaybeStaticStr causes significant indirection.

Otherwise, for downstream append, even if the key is a static str, one may have to key.to_string to resolve some lifetime issue. See:

github.com/open-telemetry/opentelemetry-rust@f7b0dd999d/opentelemetry-appender-log/src/lib.rs (L216) (they have Key::from_static_str)
github.com/slog-rs/stdlog@252b6bb643/kv.rs (L46) (they have slog::Key::from(static_str))

Replace `&'k str` in `Key<'k>` with `MaybeStaticStr<'k>` to leverage `&'static str` when in most cases it is. Open for review, especially on: 1. Whether it is backward compatible. 2. Whether `MaybeStaticStr` causes significant indirection. Otherwise, for downstream append, even if the key is a static str, one may have to `key.to_string` to resolve some lifetime issue. See: * https://github.com/open-telemetry/opentelemetry-rust/blob/f7b0dd999d3d6434a6e52291efccc98e9dfcea81/opentelemetry-appender-log/src/lib.rs#L216 (they have `Key::from_static_str`) * https://github.com/slog-rs/stdlog/blob/252b6bb6435a8a3ba44748b2e1626aa676508097/kv.rs#L46 (they have `slog::Key::from(static_str)`)

KodrAus (Migrated from github.com) approved these changes

2026-06-01 02:11:33 -05:00

KodrAus (Migrated from github.com) left a comment

This looks like a good change to me, thanks @tisonkun!

tisonkun (Migrated from github.com) reviewed

2026-06-01 02:31:25 -05:00

src/kv/key.rs

					
				@ -35,15 +36,23 @@ impl ToKey for str {

				// If a new field (such as an optional index) is added to the key they must not affect comparison

Maybe from_static. Not sure what a good name is, but both seem to be intuitive :P

View changes since the review

Maybe `from_static`. Not sure what a good name is, but both seem to be intuitive :P *[View changes since the review](https://triagebot.infra.rust-lang.org/gh-changes-since/rust-lang/log/727/761461a5d0c8ea3d483d79b1de0205c2897318d2..db145979e229549215300f2696fa89b215cb1cab)*

KodrAus (Migrated from github.com) reviewed

2026-06-01 06:28:30 -05:00

src/kv/key.rs

					
				@ -35,15 +36,23 @@ impl ToKey for str {

				// If a new field (such as an optional index) is added to the key they must not affect comparison

I think from_static_str is a good name, because it adds to the existing from_str name 👍

I think `from_static_str` is a good name, because it adds to the existing `from_str` name :+1:

KodrAus (Migrated from github.com) reviewed

2026-06-01 06:29:17 -05:00

src/kv/key.rs

					
				@ -35,15 +36,23 @@ impl ToKey for str {

				// If a new field (such as an optional index) is added to the key they must not affect comparison

Actually, looking at our other methods that use this type, they all carry a _static suffix, like module_path_static. So I think we should call this from_str_static.

Actually, looking at our other methods that use this type, they all carry a `_static` suffix, like `module_path_static`. So I think we should call this `from_str_static`.

KodrAus (Migrated from github.com) requested changes

2026-06-01 06:29:54 -05:00

KodrAus (Migrated from github.com) left a comment

As noted, we should call the method from_str_static, to match the convention of other methods in log

As noted, we should call the method `from_str_static`, to match the convention of other methods in `log`

Thomasdezeeuw (Migrated from github.com) reviewed

2026-06-01 07:45:33 -05:00

src/macros.rs

					
				@ -444,12 +444,16 @@ macro_rules! __log_logger {

				#[cfg(feature = "kv")]

				macro_rules! __log_key {

Would it be possible to use the static version here as well? I think it would be confusing that key = 42 is static and "key" = 42 is not.

View changes since the review

Would it be possible to use the static version here as well? I think it would be confusing that `key = 42` is static and `"key" = 42` is not. *[View changes since the review](https://triagebot.infra.rust-lang.org/gh-changes-since/rust-lang/log/727/761461a5d0c8ea3d483d79b1de0205c2897318d2..db145979e229549215300f2696fa89b215cb1cab)*

tisonkun (Migrated from github.com) reviewed

2026-06-01 07:46:52 -05:00

src/macros.rs

					
				@ -444,12 +444,16 @@ macro_rules! __log_logger {

				#[cfg(feature = "kv")]

				macro_rules! __log_key {

:expr is not necessary to be &'static. I'd like to know if we can distinguish the case here as well.

I'll try to investigate it later.

`:expr` is not necessary to be `&'static`. I'd like to know if we can distinguish the case here as well. I'll try to investigate it later.

tisonkun (Migrated from github.com) reviewed

2026-06-01 12:11:19 -05:00

src/kv/key.rs

					
				@ -35,15 +36,23 @@ impl ToKey for str {

				// If a new field (such as an optional index) is added to the key they must not affect comparison

This is updated.

tisonkun (Migrated from github.com) reviewed

2026-06-01 12:16:05 -05:00

src/macros.rs

					
				@ -444,12 +444,16 @@ macro_rules! __log_logger {

				#[cfg(feature = "kv")]

				macro_rules! __log_key {

756c279649 supports this by determining literal as a special case. Also tests added.

756c279649f79ce0ef8dccf952c5df4017791d1c supports this by determining `literal` as a special case. Also tests added.

Thomasdezeeuw (Migrated from github.com) approved these changes

2026-06-01 14:18:26 -05:00

KodrAus (Migrated from github.com) approved these changes

2026-06-01 15:52:52 -05:00

KodrAus (Migrated from github.com) left a comment

Thanks @tisonkun

Sign in to join this conversation.

No reviewers

Thomasdezeeuw

KodrAus

No labels

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-rust-lang/log!727

				`@ -35,15 +36,23 @@ impl ToKey for str {`
				`// If a new field (such as an optional index) is added to the key they must not affect comparison`

Rows
Columns