Trace Payload Collection

[nirga]

Continuing the work started by @ronyis at #219.

We're proposing semantic attributes for reporting payloads (of HTTP requests, DB queries, etc.) as part of OpenTelemetry. This should be optional for users who wish to use it and can provide consumers of OpenTelemetry with a standard option for getting payloads out of trace instrumentation.

Previous instrumentation libraries (pre-otel) have successfully implemented it so I think this is something that will be welcomed by the community.

Would love to get your feedback and work on getting this accepted.

[jsuereth]

Since we use protocol buffers, the size/length of content is implicitly recorded.

I believe even for JSON this is effectively true (you'll have a length/size available at runtime) so no need to encode it separately.

[nirga]

I think it's important to keep the size as a separate attribute, since we may have an actual different content size (for example if we truncated part of it).

[jsuereth]

Are you going to allow "versioning" of these protocols?

What's the path for adding new (custom) encodings?

[nirga]

Note that this is only the encoding originally initially when sending the payload. The content structure inside the span should always be the same, regardless of the protocol. This is actually another reason why we should translate the payload into some common structure as proposed here.

[jsuereth]

Since this was added in real tracing products, can you link to the APIs/SDKs they offered?

[nirga]

It's a bit different, since they just offered this instrumentation OOTB, so the API for the payload was hidden from the developer. See for example Epsagon's instrumentation for FastAPI (The exact structure of the data is seen here)

[jsuereth]

+1 to resolving this discussion as a priority.

[nirga]

But should it affect the attribute names for spans as proposed here?

[jsuereth]

Where is this supported in OTel proto schema? Do you mean the "AnyValue" being allowed to be "emtpy"?

I'd like more details about where/how you need Null support here. Are you trying to record payload attributes even if the payload is null? Could you just write an empty byte array for the Proto/Avro case?

[nirga]

Yes. Hmm, this can work. Updated this.

[ronyis]

The point here is that we need to support JSON payloads like '{"x": null}' and also null, and similar examples in other formats.
If the payload is encoded with the "AnyValue" type, it can be achieved by using empty objects. Though it's still needed to be supported by the APIs.
In case we add a dedicated proto object, like the Value suggested in the alternative, we should also make sure it supports Null values.

[jsuereth]

I'd need a lot more justification on a new AnyValue type in OTLP before I'd be convinced. I think we already support the JSON payloads you mention.

[jsuereth]

I think it's worth calling out that payload collection likely requires some kind of flag to signal that sensitive data collection is ok.

Specifically - I think this proposal needs some kind of notion on a per-trace basis that denotes whether sensitive collection is "allowed" for that trace.

E.g. you can imagine a system where I issue an RPC with a special flag denoting that collection of payloads (and other sensitive data) is ok, where this would be turned on.

You can also imagine production systems that require user-consent before this data could be collected in a o11y system. We should absolutely have a mechanism to check (dynamically, per-request / per-trace) whether this data can be collected.

[nirga]

I wonder if this might be out of scope for this as it should be resolved on the API level of the different SDKs. Here I suggest something much simpler - if we ever to auto-instrument collection of payloads, the default should always be off where users who decide to turn it on should know it will be turned on for everything.

[ronyis]

I'm also in favor of having this kind of flag regarding sensitive data collection. Though I don't think it should be a requirement for the changes proposed in this OTEP.
At this point, users already collect sensitive data using the existing APIs and attributes.

[mishushakov]

Would love to see that happen at Step CI 😉

[axlrate]

Should the payload be a span event instead of a span attribute? Most backends have higher size limits for span events.

[mmanciop]

Should the payload be a span event instead of a span attribute? Most backends have higher size limits for span events.

Lumigo does this with span attributes, and the experience was that a truncation to 1024, 2048 or 4096 or power thereof was often good enough. (The complex bit was performing truncation that would keep validity of the data format, e.g., JSON.) Users have means via SDK configuration to change the attribute truncation threshold. I think setting this data as span events would be very unintuitive in terms of data model. I don't think we should use an unintuitive data model to work around limitations of backends. Moreover, for an important use-case like this, I can imagine backends makings exceptions for the specific keys we'd use.

[codefromthecrypt]

might be worthwhile to ping folks who did some things in this space, even if moved on since.

• @mchandramouli who created blobs at expedia for request/response logging. There are some ancient notes here, but I suspect concepts are still relevant.

• @pavolloffay who worked on the hypertrace java agent which adds request/response collection.

[carlosalberto]

OTEPs have been moved to the Specification repository. Please consider re-opening this PR against the new location. Closing.