add more LogfireConverter

[gabrielhsc95]

This adds more LogfireConverter to address #58, and many other types.

I also took the liberty to update the test of the macro a bit, to avoid repetition, and added tests for many rust types for log!

The examples also update to show that you can actually use any time.

[gabrielhsc95]

The first thing I noticed, it that just simply trying to implement the Trait was not possible because of the orphan rule (Value is in opentelemetry and From is in std, and I tried implementing that in logfire).

So then, I tried changing it from the opentelemetry, but that required changing the proto file, which sounded a bit too much, and I also noticed that converters would be necessary, because Rust have more primitive types than proto.

Finally, I tried LogfireConverter as same as it is done for usize, but for that I created a new struct called IntoI64Value that would be implemented for many primitives, including usize, so then I could use that for the conversion.

I am aware that this is probably not the most efficient algorithm, because in the end I have to convert a variable twice, once to a String, and second parsing it into a i64. Before, the previous would only convert to String when necessary.

Similar to the previous implementation, in case of overflow, the variable is actually a String in the end.

The issue #58, only mentioned u32, but I did for u8, u16, u32 (initial issue), u64, u128, usize (re implemented), i8, i16, i32, i128, isize. Once I was already there I also implemented for f32 and char, those were using the same idea of the initial LogfireConverter, without having to introduce a new struct.

To be very thoroughly, I also added some tests, but I noticed the a lot of the tests were repeating

let output = Arc::new(Mutex::new(Vec::new()));
let console_options = ConsoleOptions {
    target: Target::Pipe(output.clone()),
    ..ConsoleOptions::default().with_min_log_level(Level::INFO)
};

let logfire = crate::configure()
    .local()
    .send_to_logfire(false)
    .with_console(Some(console_options))
    .with_default_level_filter(tracing::level_filters::LevelFilter::TRACE)
    .finish()
    .unwrap();

let guard = crate::set_local_logfire(logfire);

so I added that to a helper function.

The final change was to update the examples, where no more casting was required.

[davidhewitt]

Thanks for the PR, really appreciate the help!

I would strongly prefer we try to use try_from first, because it'll be cheap to fail to create an i64 from an out-of-bounds integer.

Going unconditionally via strings and then attempting to parse them will probably introduce major overheads for users using logfire to capture simple numeric values.

[davidhewitt]

Maybe let's use debug format so that the fact these are being sent as strings is captured in the test.

Suggested change

	crate::info!("an i128: {value}", value = 10i128);
	crate::info!("an i128: {value:?}", value = 10i128);

[gabrielhsc95]

The only one that actually changes is the char:
from an char: a to an char: 'a'

The number cases are the same with and without :?

[davidhewitt]

Why was i64::try_from no longer possible? I think all of these types should implement that conversion.

Maybe rather than a generic on T (those can be problematic, we only really ever are allowed one) can we do this with a macro that adds a concrete implementation?

e.g.

impl_into_i64_value!(usize);  // expands to an implementation for `LogfireConverter<usize>`

[gabrielhsc95]

I tried it again, and remember why I did something else. So the problem is that the type become ambiguous for the compiler, so cases like:

let _ = log!(Level::INFO, "int_attr_log", num = 42);

which can be fixed by let _ = log!(Level::INFO, "int_attr_log", num = 42i64);

Therefore, for those types of test case, I change to i64 when ambiguous. I chose i64 since it is the default from open telemetry.

Another small issue is that for the cases that don't overflow like u8, u16, u32, i8, i16, i32, the compiler complain saying: note: this pattern will always match, so the if let is useless.

To solve that, then I created the following two macros: impl_into_from_i64_value and impl_into_try_into_i64_value.

[gabrielhsc95]

I fixed the doc tests. I also have to admit that not all tests pass on cargo test, but I doesn't work for me even in main. But I assumed it is something about my setup, since the failed tests are the same in this branch and in main. Those are the tests that failed.

failures:
logfire::tests::test_credentials_file_error_handling
logfire::tests::test_credentials_file_loading
logfire::tests::test_no_credentials_file_fallback

[codecov-commenter]

Codecov Report

❌ Patch coverage is 96.55172% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/macros/impl_.rs	80.00%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!