[maxday]

✍️ Description of changes:

Adds context.w3c() which exposes allowlisted W3C trace-context fields (traceparent, tracestate, baggage) carried on clientContext.w3c, and strips w3c from clientContext so callers can only read trace fields through the helper.

🔏 By submitting this pull request

• I confirm that I've ran cargo +nightly fmt.
• I confirm that I've ran cargo clippy --fix.
• I confirm that I've made a best effort attempt to update all relevant documentation.
• I confirm that my contribution is made under the terms of the Apache 2.0 license.

[darklight3it]

Here I think you can simplify significantly and drop the mutability.

There's no need to pop w3c out of the client context: ClientContext doesn't declare that field, and since it doesn't use deny_unknown_fields, serde just ignores it without erroring.

So client_context_value doesn't need to be mut, and the helper can take a plain &Value, letting the caller handle the Option:

let w3c_fields = client_context_value
    .as_ref()
    .map(Self::w3c_fields_from)
    .unwrap_or_default();
let client_context: Option<ClientContext> = client_context_value
     .map(serde_json::from_value)
     .transpose()?;

/// Read the allowlisted string fields (see `W3C_ALLOWED_FIELDS`) from
/// `clientContext.w3c`; anything else is dropped.
fn w3c_fields_from(client_context: &serde_json::Value) -> HashMap<String, String> {
    let w3c = &client_context["w3c"];
    W3C_ALLOWED_FIELDS
        .iter()
        .filter_map(|key| Some((key.to_string(), w3c.get(*key)?.as_str()?.to_owned())))
        .collect()
}

&client_context["w3c"] yields Value::Null so all the invalid shapes fall out of the filter_map. Since from_value still consumes the Value, the ClientContext strings are moved rather than copied, so there's no extra cost compared to the current code.

transpose it's what you can use in place of

match client_context_value {
    Some(v) => Some(serde_json::from_value(v)?),
    None => None,
}

[darklight3it]

I would not return a copy here. This is not very idiomatic for Rust.

I think you should return a borrow here.

pub fn w3c(&seld)  -> &HashMap<String, String>{
   &self.w3c_fields
}

In this way you have compile time guarantee of immutability. If the customer wants to mutate the hashmap they are forced to do something like.

ctx.w3c().clone().

Leaving to them the choice of whether to copy or not.

Doing that will probably make one of your test not compile.

[darklight3it]

The strips_source part is not tested and also it's not important because the client_context ignores it.

[darklight3it]

The W3c standard explicitly tells to preferably any case for all three of them. But it refers specifically to haders.

This is contentious but I think we should go in this direction. Had problem in the past with this kind of issues.

• traceparent header name (§3.2.1): https://www.w3.org/TR/trace-context/#header-name. It says: "Vendors MUST expect the header name in any case (upper, lower, mixed), and SHOULD send the header name in lowercase."
• tracestate header name (§3.3.1): https://www.w3.org/TR/trace-context/#header-name-0. The wording is the same.
• baggage header name (§3.1): https://www.w3.org/TR/baggage/#header-name. It only says "implementations SHOULD keep the header name lowercase." There's no "MUST expect any case" rule for baggage.

[darklight3it]

We should also not accept tracestate if traceparent is not set but in this case I can understand that you are doing this because you want to defer this kind of validation to OTEL SDK code.