Otel improvements

[ramonsmits]

No description provided.

[ramonsmits]

I don't think it should always use base64. If the payload if text, then it should just be utf8.

Second, we should NOT do extra (de)serialization. We should do this where we already have access to byte[] and then write this either as utf8 string or base64 string. To decide that we have probe for \0, force if via config (log payload as base64 or utf8), or assume utf8, catch exception on GetBytes and fallback to base64 encoding, maybe even another option.

[danielmarbach]

FYI As part of the assembly scanning efforts we have tried to sort as much as possible the paths that require dynamically referenced code. The JSonSerializer usage here would regress on that because it introduces another method usage that is not trimming friendly

[System.Diagnostics.CodeAnalysis.RequiresDynamicCode("JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. Use System.Text.Json source generation for native AOT applications.")]
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.")]
public static string Serialize(object? value, Type inputType, System.Text.Json.JsonSerializerOptions? options = default);

we are planning to add some kind of "regression" approval soon that should capture things like that in the future

While I'm here, I would like to add something else. I’m concerned about adding the full message body as an activity tag.

The OTel messaging semantic conventions define messaging.message.body.size, but that is only the size of the body in bytes, not the body content itself. I don’t see a standard semantic convention for attaching the full message payload to a span attribute.

Adding the full body as nservicebus.message.body, even base64 encoded, seems risky:

• high-cardinality span attributes
• potentially large telemetry payloads
• possible PII/secrets leakage
• increased indexing/storage cost in observability backends
• base64 does not provide sanitization

But I'm not in the weeds as you are so I might miss important context.

[danielmarbach]

FYI #7800

[ramonsmits]

THis should not use magic strings

[ramonsmits]

This likely should be in ActivityDecorator

[ramonsmits]

Shouldn't we append the incoming message type? Like the 1st type from EnclodedMessageTypes?