open-telemetry / oteps

OpenTelemetry Enhancement Proposals
https://opentelemetry.io
Apache License 2.0
337 stars 164 forks source link

Restrict LogRecord Body to string in Logging SDKs #188

Closed djaglowski closed 2 years ago

djaglowski commented 2 years ago

Discussions in the Logs SIG have reached consensus that the Body of a LogRecord should be restricted in Logging SDKs.

The logs data model technically allows the Body to be arbitrarily structured, but this is only for the sake of preserving the semantics of legacy log formats.

It is intended that the Attributes field should be used for all structured logs and the Body should contain only a string message.

Resolves (specification) #2068

yurishkuro commented 2 years ago

While I agree with the spirit of the proposed change, I don't understand its normative application. SDKs accept data provided to them via logging API. Logging APIs are not in scope of OTel today. So what does it mean for SDK to restrict body to string?

If we're talking about the API exposed by the SDK itself (i.e. the API that would be used by a translator that bridges data from a 3rd party Logging API into OTel model), then why are we phrasing this with lower-case "should" instead of MUST?

djaglowski commented 2 years ago

While I agree with the spirit of the proposed change, I don't understand its normative application. SDKs accept data provided to them via logging API. Logging APIs are not in scope of OTel today. So what does it mean for SDK to restrict body to string?

If we're talking about the API exposed by the SDK itself (i.e. the API that would be used by a translator that bridges data from a 3rd party Logging API into OTel model), then why are we phrasing this with lower-case "should" instead of MUST?

I am admittedly a bit unsure how to most appropriately frame the statement. Informally, I think we effectively want the SDKs to operate as if the data model has defined the body as a string. Arguably, this is already stated by the language recently added to the data model's definition of the body, First-party Applications SHOULD use a string message.

Perhaps it's worth stopping here to validate whether or not any additional language is necessary. WDYT?

If it is necessary to clarify that the SDKs should have special consideration for the body, does it make sense to frame it as an effective restriction on the data model, or do you think it's more appropriate to pose it in terms of the API exposed by the SDK?

tigrannajaryan commented 2 years ago

While I agree with the spirit of the proposed change, I don't understand its normative application. SDKs accept data provided to them via logging API. Logging APIs are not in scope of OTel today. So what does it mean for SDK to restrict body to string?

We have an OTEP that defined Logging Library SDKs, which exposes the Body of the Log Record: https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md This limitation will apply to the Logging Library SKDs.

djaglowski commented 2 years ago

This was discussed in the Log SIG today, and it was agreed that recently added language in the data model sufficiently captures the intent of the Body field.