Open nblumhardt opened 1 year ago
This should be addressed before stable release, so adding to that milestone.
https://github.com/open-telemetry/opentelemetry-dotnet/pull/4781/files Has removed exporting eventid completely to unblock stable release. Removing this issue from 1.6 milestone, but this issue is still open and need fix.
https://github.com/open-telemetry/opentelemetry-dotnet/pull/4925#discussion_r1348943492 There is a desire to not use any language specific attribute name, instead propose this as an OTel Sem. Convention, so every other language can leverage this (only OTel CPP needs this currently. OTel Rust would need it in the future as well).
How about just event_id
and event_name
?
There is an OpenTelemetry Events API that is related to Logs.
An Event is a specialized type of LogRecord, not a separate concept.
Which seems to line up - a log record with an event id/name is an event.
This defines event.name
in its semantic conventions
https://opentelemetry.io/docs/specs/semconv/general/events/
While it's not stable yet, it seems like a reasonable name for it, and event.id
could be used as well but is not defined by the spec.
The current names, hidden behind the OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EVENT_LOG_ATTRIBUTES
option, are logrecord.event.name
and logrecord.event.id
, so this would just remove the logrecord.
prefix from those.
It looks like these current names were chosen just to unblock it and move forward and it also looks like there are plans to stabilise these so that they can be safely used but that seems to be making a temporary decision permanent without revisiting what it should be long term, and it seems like these names should really be OTel semantic conventions.
@ghelyar you are right. Leveraging Events convention was one of the suggestions. (Note that the whole notion of Events was not very clear until recently, so initially, there was reluctance to follow events convention. With recent clarification on Events purpose, it needs a revisit.)
Stabilizing the feature is not simply emitting the current values by default. It is figuring out the correct way to do it and then making that the default. The right way could very well be based on Event semantic convention.
Thanks for explaining @cijothomas
For what it's worth and in case it helps anyone else, this is what I'm currently doing in a processor that is added before the OTLP exporter, although it's a bit long winded due to LogRecord.Attributes
being an IReadOnlyList
:
internal sealed class EventIdLogProcessor : BaseProcessor<LogRecord>
{
public override void OnEnd(LogRecord data)
{
var attributes = new List<KeyValuePair<string, object?>>(2 + (data.Attributes?.Count ?? 0));
if (data.EventId.Id != default)
{
attributes.Add(new KeyValuePair<string, object?>("event.id", data.EventId.Id));
}
if (!string.IsNullOrEmpty(data.EventId.Name))
{
attributes.Add(new KeyValuePair<string, object?>("event.name", data.EventId.Name));
}
if (attributes.Count > 0)
{
if (data.Attributes is not null)
{
attributes.AddRange(data.Attributes);
}
data.Attributes = attributes;
}
}
}
Thanks. The above approach, which works, forces another List heap allocation/copying and is not acceptable for perf sensitive apps. :(
The open issue about semantic convention for a event.id field. Most discussions occurred on this while Events was unclear, so this also requires re-discussion! https://github.com/open-telemetry/semantic-conventions/issues/372
Feature Request
EventId
is part of .NET's stable log data model. Log events carry an event id, which may contain anId
for the log event (type), and an additional human-readableName
.Currently, the OTLP exporter flattens these structure members into
Id
andName
attributes on resulting log records. These are extremely common attribute names to find in user log messages, for example:generates two important attributes on the resulting log event,
Name
andId
, yet these will conflict with theEventId
-derivedName
andId
attached by the exporter.Describe the solution you'd like:
Something akin to the way category names are handled could work? I.e.:
Describe alternatives you've considered.
--
Additional Context
See also https://github.com/open-telemetry/opentelemetry-dotnet/issues/3491