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

Align Embedded Logs data model with Standalone Logs data model #622

Open
tigrannajaryan opened this issue May 26, 2020 · 6 comments
Open
Labels
area:semantic-conventions Related to semantic conventions release:after-ga Not required before GA release, and not going to work on before GA spec:logs Related to the specification/logs directory

Comments

@tigrannajaryan
Copy link
Member

tigrannajaryan commented May 26, 2020

Now that we have an official standalone logs data model it will be beneficial to see if we can align logs embedded in the Spans (Span events) with this data model.

Embedded spans should use the same or similar model except the following, which are already recorded in the containing Span:

  • Trace context fields: trace id, span id, trace flags.
  • Resource.

The differences currently are:

Span Event Standalone Log Record Notes
Name Name Equivalent semantic, only field names are different.
- Severity Missing in Span Event.
- Body Missing in Span Event.
Attributes Attributes Almost equivalent, but Standalone Logs allow complex nested values that Span Events don't.

We need to decide which of these differences are acceptable/necessary. My preliminary thoughts are the following:

  • Rename Name to ShortName. Rename ShortName to Name. There is no semantics change. DONE.
  • There is no need to add Body to Span Events. For standalone logs the Body is necessary to record what cannot be represented in the attributes or other fields for log formats that have unusual/unknown requirements. There are no such requirements for Span Events because we know they are generated (using OpenTelemetry API).
  • We should extend Span Event Attributes to allow the same nested values. This was requested/discussed in the past too.
  • I am torn on Severity. We should use it if we believe the Error Semantics it introduces are applicable to Span Events.
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-proto that referenced this issue May 26, 2020
This is part of the alignment effort defined here open-telemetry/opentelemetry-specification#622

The comment added clarifies the semantics, but I believe this is compatible
with our previous understanding and doe not change that understanding.
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-proto that referenced this issue May 26, 2020
This is part of the alignment effort defined here open-telemetry/opentelemetry-specification#622

The comment added clarifies the semantics, but I believe this is compatible
with our previous understanding and doe not change that understanding.

This does not break the protocol since renaming the fields does not change
the wire format.
@Oberon00
Copy link
Member

  • Body can be a semantic convention for attributes.
  • Severity can be a semantic convention for attributes
  • ShortName could be renamed to Name.
  • Why are nested attributes needed for logs? Is this not cases where you could do JSON in / JSON out?

@tigrannajaryan
Copy link
Member Author

ShortName could be renamed to Name.

I would like to know what others think. This seems reasonable to me.

@tigrannajaryan
Copy link
Member Author

Body can be a semantic convention for attributes.

Yes, it can. The question is: do we need a Body for Span Events? Our API does not have anything like that and I do not think we need to add it to the API (but perhaps there is a good use case that I am not aware of).

@tigrannajaryan
Copy link
Member Author

Severity can be a semantic convention for attributes

I agree, it can. However, I think there is a value in consistency and uniformness. Making it a field in Log Data model and a semantic convention in Span Event is somewhat inconsistent.

@tigrannajaryan
Copy link
Member Author

Why are nested attributes needed for logs?

This came up several times in the past in OpenTelemetry, not just for logs, but for Spans too. I think it is time to make it possible.

Is this not cases where you could do JSON in / JSON out?

We use strong typing for attribute values. There is no way to know that a string attribute is a JSON value that needs to be decoded to be understood.

@tigrannajaryan tigrannajaryan added the spec:logs Related to the specification/logs directory label May 27, 2020
tigrannajaryan pushed a commit to tigrannajaryan/rfcs that referenced this issue May 28, 2020
Contributes to open-telemetry/opentelemetry-specification#622

This aligns the naming to what is already used for Embedded Logs (Span.Event).
There is no change of semantics.
carlosalberto pushed a commit to open-telemetry/oteps that referenced this issue Jun 4, 2020
Contributes to open-telemetry/opentelemetry-specification#622

This aligns the naming to what is already used for Embedded Logs (Span.Event).
There is no change of semantics.
@bogdandrutu bogdandrutu added the area:semantic-conventions Related to semantic conventions label Jun 26, 2020
@reyang reyang added the release:after-ga Not required before GA release, and not going to work on before GA label Jul 6, 2020
@marcelocra-hubla
Copy link

Hi folks! Is this something you are still planning on doing? Tks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions release:after-ga Not required before GA release, and not going to work on before GA spec:logs Related to the specification/logs directory
Projects
None yet
Development

No branches or pull requests

5 participants