Do the TelemetryAPI records support JSON record fields?

[mheffner]

According to the TelemetryAPI docs, if you're using a more recent schema version than "2022-12-13", the record field of function and extension logs may be a string OR a JSON record.

From the code it would appear that these only support the string records. Do these need to be updated to something like serde_json::Value to handle both cases?

https://github.com/awslabs/aws-lambda-rust-runtime/blob/f69280e9b92e9a7acc92cfa04a860d9b2b83b5c0/lambda-extension/src/telemetry.rs#L25-L30

Thanks!

[jlizen]

Looks like you're right! Feel free to modify this issue into a feature request, or take a stab at implementing it!

One way to deal with this might be to encode it at the type level via a generic (or more likely a separate struct, since adding a generic to LambdaTelemetryRecord would cause semver breakage). And then the caller could just specify that in their handler if they want to receive processed JSON. The downside of that approach though is that it would be effectively hardcoding a choice around log format, whereas advanced logging controls are designed to be changeable in console, etc. I guess probably the caller could write two different handlers and choose between them based on the same ENV, but it's all pretty klunky.

Maybe it would be better to newtype both of the inner record types and offer a .json() method on them that lazily attempts to deserialize if invoked? That keeps the handler signatures simple but still lets the caller apply context around whether a given record format is set or not?

You could MAYBE hack something together with a new typed inner enum that has an internally tagged variant for serde that uses one of the known json field names, and a #[serde(other)] tag on the plain variant? Except that would force callers to match on them all the time, and also that would be semver breakage.

I think probably .json() would be simplest to implement + less terrible to maintain / not too unergonomic for callers...?

Just sharing my perspective! (Not a maintainer / not offering to implement this)

[mheffner]

That all makes sense @jlizen. Yeah, we decided to go with a quick fix to swap these for serde_json::Value types. Obviously, we weren't trying to maintain semver compatibility and it pushes the match handling onto the user. I'm happy to open a PR with that, but as you allude to, I think this is probably a decision for the maintainers as to how they'd want to proceed.

[jlizen]

cc @bnusunny for perspective on this one... my knee jerk is, probably not good to cut to modeling a serde_json::Value type, even across a breaking semver boundary. Seems like a future semver hazard and anyway it's trivial for callers to choose to parse the String into JSON themselves. If we did want to do it, we also probably would want to make it a RawValue to avoid extra cost if caller is just passing it through without interacting with fields.

That said I do think a .json() method (or if this is a strongly typed schema, a .record() API that parses into it) might be good as a middle ground to help with ergonomics.

[jlizen]

@mheffner what do you think about the generic being discussed in #1098 ?

It lets you specify:

async fn handler(events: Vec<LambdaTelemetry<serde_json::Value>>) -> Result<(), Error> {
   println!("{events:#}");
    Ok(())
}

#[tokio::main]
fn main() {
    let telemetry_processor = SharedService::new(service_fn(handler));

    Extension::new()
        .with_telemetry_processor(telemetry_processor)
        .run()
        .await.unwrap()
}

but still defaults to String if unspecified. (Probably we should default to json next major version... this fell off the radar for the 1.0 bump sadly)

[mheffner]

Hey @jlizen, I believe that generic approach would work fine for us. We are not using the Extension runner nor telemetry_processor in that manner, so our use case is simpler. The generic would work in that model:

https://github.com/streamfold/rotel-lambda-extension/blob/af6de2fcf033ee5fe8a3dbc060eb921c7f15d756/src/lambda/telemetry_api.rs#L196-L198

I can't talk to the semver breakage, we'd likely be fine bumping a major/minor version if it was required and there weren't other breaking changes.

[jlizen]

This will be going out next release! This is a good issue to keep tabs: #1108

Here's the ultimate API we landed on, that avoid basically all breakage (besides some niche inference cases):

    /// use lambda_extension::{Extension, LambdaTelemetry, SharedService, service_fn};
    ///
    /// async fn handler(events: Vec<LambdaTelemetry<serde_json::Value>>) -> Result<(), lambda_extension::Error> {
    ///     for event in &events {
    ///         println!("{event:?}");
    ///     }
    ///     Ok(())
    /// }
    ///
    /// let _ext = Extension::new()
    ///     .with_telemetry_record_type::<serde_json::Value>()
    ///     .with_telemetry_processor(SharedService::new(service_fn(handler)));

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.