Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict LogRecord Body to string in Logging SDKs #188

Closed

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Nov 16, 2021

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

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.
@djaglowski djaglowski marked this pull request as ready for review November 16, 2021 15:28
@djaglowski djaglowski requested review from a team November 16, 2021 15:28
@yurishkuro
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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:/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md
This limitation will apply to the Logging Library SKDs.

@djaglowski
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit Body to string in logging library APIs
3 participants