Open bogdandrutu opened 2 months ago
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-attributes already states that, Log Attributes are a superset of Attributes allowed in Span,Metrics, and that was intentional.
Whether Event API should support the supertset or not: Its being discussed in https://github.com/open-telemetry/opentelemetry-specification/issues/4199.
@cijothomas thanks for the link. I think the word "superset" does not imply that the "standard attributes" must be allowed in the LogRecord
attributes or body. At least some SIGs did not understand that, see the golang example or the new Java Value where the "setBody" does not allow me to use the Attributes https://github.com/open-telemetry/opentelemetry-java/pull/6591/files#diff-7adba3bfe9b75261ce86c136c33a1e3f01a427c3ac7a378213567fc2d841f8aaR78
At least some SIGs did not understand that, see the golang example
We can always add a function converting a standard attribute to log attribute if it would occur to be needed.
EDIT: I created https://github.com/open-telemetry/opentelemetry-specification/issues/4202
the new https://github.com/open-telemetry/opentelemetry-java/pull/6591 where the "setBody" does not allow me to use the Attributes https://github.com/open-telemetry/opentelemetry-java/pull/6591/files#diff-7adba3bfe9b75261ce86c136c33a1e3f01a427c3ac7a378213567fc2d841f8aaR78
The body is type AnyValue
, not a Map<String, AnyValue>
like attributes. The java setBody
accepts a Value
type (i.e. the java type for AnyValue
) to reflect this. Allowing the body to have attribute key / value pairs added to it with something like addToBody(AttributeKey<T> key, T value)
is saying two things: 1. I want the body to be a AnyValue type of key value list. 2. I want to add an entry to the key value list.
Given that the log bridge API is not intended to be user facing, API ergonomics are not a high priority. After all, how often do you need to reference standard attributes when writing a log appender?
Note that in java we did add good support for standard attributes to the Event API prototype: https://github.com/open-telemetry/opentelemetry-java/blob/e063b34a2ad1865b8e8bc0e8758e6c375efbf473/api/incubator/src/main/java/io/opentelemetry/api/incubator/events/EventBuilder.java#L79-L114
The difference being:
Note that this isn't necessarily the final say on the java log API. Its possible that we add syntactic sugar to make it easier to add standard attributes to the body like we've done with the Event API.
Java is not aligned with the spec requirement that log attributes (i.e. not the body) are Map<String, AnyValue>
. We could add support for that, but nobody has asked. If this is important to you, please open an issue and describe the use case.
Whether Event API should support the supertset or not: Its being discussed in #4199.
This implicitly conflicts with this:
Given that the log bridge API is not intended to be user facing, API ergonomics are not a high priority. [...] The event API is user facing so ergonomics are important.
If it follows that a public API should support the common attributes, then we need a way to convert to log attributes given the current design of the events API.
Otherwise, if we decide that the public Events API doesn't support common attributes, the argument that it's okay because it is only a bridge becomes invalidated.
Java is not aligned with the spec requirement that log attributes (i.e. not the body) are Map<String, AnyValue>. We could add support for that, but nobody has asked. If this is important to you, please open an issue and describe the use case.
I think Java actually did the right thing keeping the log attributes as "standard attributes" because these are used to "identify a log entry". I actually want everyone for Attributes to use the "standard attributes" but for body we can have the "Any" value.
@jack-berg depends how you read:
The log attribute model MUST support any type, a superset of standard Attribute, to preserve the semantics of structured attributes emitted by the applications. This field is optional.
From your comment:
Allowing the body to have attribute key / value pairs added to it with something like addToBody(AttributeKey
key, T value) is saying two things: 1. I want the body to be a AnyValue type of key value list. 2. I want to add an entry to the key value list.
I want the KeyValue
in java to allow me to use the "standard attributes" there. Why? Because per the comment that any is a superset I should be able to use the more restrictive set there.
Notes from SIG meeting https://github.com/open-telemetry/opentelemetry-specification/issues/3849#issuecomment-1917415282:
It may sense to have log attributes to be modeled differently than standard attributes as the purpose of Logs Bridge API is to support what existing structured logging libraries offer. Some applications use complex values for attributes and end-users would like to keep them.
I want the KeyValue in java to allow me to use the "standard attributes" there. Why? Because per the comment that any is a superset I should be able to use the more restrictive set there.
This is totally possible in Java. See see my previous comment:
Given that the log bridge API is not intended to be user facing, API ergonomics are not a high priority. After all, how often do you need to reference standard attributes when writing a log appender?
What are you trying to achieve?
Share the same attributes across traces/metrics/logs. I do understand that the logs.attributes (this was a surprise that even attributes are different) and especially the body (not surprised about this) can be more generic than the traces and metrics, see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-attributes and https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-body.
What did you expect to see?
Clear requirement in the bridge-api to support adding the "standard attributes" as well, since there are semantic conventions generate (example in go) that use the old style attributes.
Additional context.
Currently because of the logs data model saying that log record attributes are different than span/metrics attributes and that the bridge api reference to that https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/bridge-api.md#emit-a-logrecord
Java offers only support to add the "old style" attributes as attributes, see https://github.com/open-telemetry/opentelemetry-java/blob/main/api/all/src/main/java/io/opentelemetry/api/logs/LogRecordBuilder.java
Go offers only the more generic way, and I cannot share the attributes easily, see https://github.com/open-telemetry/opentelemetry-go/blob/main/log/record.go#L112