-
Notifications
You must be signed in to change notification settings - Fork 779
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
API Proposal: Support for OpenTelemetry Events #5891
Comments
@samsp-msft what's the right area label for this issue? |
Why would this need to be a new library rather than adding to one of the existing ones? |
Events are functionality that is defined by OpenTelemetry and while event data will come through ILogger, their output is not going to be nearly as understandable if the collector is not aware of them. As its specific to OTel, I am not sure we want to put it in the ILogger libs, nor require consumers to pickup the R9 extension lib. Anyvalue is a OTel defined distributed union. We need a definition that can be used by the Events code, that can then be referenced by the OTel libraries. I don't think we can take a dependency in .NET to OTel, which is on a completely different schedule, ownership etc. |
OTel defines Events as Logs with EventName. ILogger does not restrict the types of attributes (its objects). It is upto providers like OpenTelemetry to support more complex attributes. I don't think this it is a good idea to introduce a totally new thing, just to support Events, which looks to be already supported by ILogger. If there are limitations, then is it possible to look at fixing them in the ILogger itself? (I am no longer a maintainer for OTel .NET, so my comments may not be based on most up-to-date information! |
From OTel Logs/Events side we're currently in process of updating a few things:
We'll migrate existing events to use attributes instead of the body. The body use-case remains unclear. Given that this work is not complete, there is a small(?) chance we'll have to come back to the body, especially for large/binary/opaque things not defined by OTel. How I think it applies to .NET:
What's still missing:
The latter is where the Given that things are in flux around the body and it probably won't be necessary, I think we should focus on the attribute/tag/parameter type. Here we can either:
|
The lack of body which is a key part of the logging concept feels kind of broken when using the existing APIs. What you have to do today is kind of ugly. Eg extensions/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/OpenTelemetryChatClient.cs Line 497 in 2f973c9
This is where having more knowledge of the types that can be passed into AnyValue comes into play. Ideally when you pass in values into the APIs that take an object, it will validate those against what is packable into an AnyValue, so you get an error in the code that emits the log message, rather than later when OTEL is trying to emit it.
Having AnyValue support "in the extensions" so that apps can formulate their richer bodies using AnyValue I think is going to be a more robust developer experience than stuffing values into |
If we're considering new API, I think there are a few opportunities to improve things to make them more idiomatic to OTel Events and friendly to devs:
So If we take into account all these, my naive minimally-useful API draft looks like WriteEvent(this ILogger, string fullyQualifiedName, IEnumerable<KeyValuePair<string, AnyValue>> tags) the most advanced overload WriteEvent(this ILogger,
LogLevel? level,
EventId(*) eventId, // (*) modified to encourage fully-qualified human-readable name
IEnumerable<KeyValuePair<string, AnyValue>>? tags,
AnyValue? body, // there could be an overload that takes Func<IEnumerable<KeyValuePair<string, AnyValue>>?,Exception?,string> formatter to write string body.
Exception? ex) |
Updated the text after the meeting today. @lmolkova @cijothomas @noahfalk @tarekgh, please take a look. |
Throwing in one more difference between This is not that important for traditional logging since 1) log record schema is rarely formally defined/documented 2) in general, logs don't have stability guarantees 3) logs are not globally identifiable. OTel Events are intended to be all of this. So another addition to consider is having |
[Issue writing in progress]
Add support for OpenTelemetry Events
Events is a newish spec from OpenTelemetry to describe events. The goal is to replace the events on Spans, and be able to use them for client-side events (such as a tracking button clicks), gen_ai telemetry etc.
Events are a specialization of log messages, and have a different schema:
Event data should be structured data as part of theBody
which should be transmitted on the wire using AnyValue.AnyValue is a distributed union type defined in the gRPC protos for OTLP. It has a lot of commonality with what can be used in JSON - the main difference is more options for numbers.
How Events should be exposed to .NET
Proposals
Extension to ILogger for Event API
This can be used for OTel Events or other scenarios that require a bit more control over the data format that is emitted.
It will need to have a marker in the TState (or a custom TState implementation) so that existing logging providers can decide what to do with the data.
The format provider should output all the properties as JSON.
The existing logging providers should be updated to have specialized behavior based on the TState marker:
The primary scenario for using this API should be new scenarios, so it should not be a break for existing applications.
Location: TBD
Arguments can be made for it belonging in:
Microsoft.Extensions.Logging
- the API does not have dependencies on OTel, and is just providing an alternative set of fields for loggingMicrosoft.Extensions.*
nuget package, such asMicrosoft.Extensions.OpenTelemetry
orMicrosoft.Extensions.Logging.Events
For those libraries it could be argued as to whether they belong in the
dotnet/runtime
repo ordotnet/extensions
. The advantage of the latter is that it can ship more regularly.Strongly typed Event method generation update - Parameter name mapping
Logging since .NET 7 has supported the
LoggerMessage
attribute to create optimized logging methods to write standard log messages. Because the parameters to this are already optional, it can be used for OTel Events without requiring changes.Issue: If
Body
comes back for Event definition, then it should be added as an option to theLoggerMessageAttribute
as another optional parameter.One of the main problems with this API is that the name of attributes in the resulting log message will be taken from the names of the parameters to the method (eg
foo
andbar
in the above example). This limits the names that can be used, especially for OTel which commonly requires "a.b.c" format names for its semantic conventions. If .NET customers are using this API to implement OTel standard events, then they need to be able to map the parameters to the method to potentially different names in the resulting OTel output.We can solve this problem in a similar way to how the R9 extension handles object parameters - by enabling attributes on the method parameters:
In the above example, the
foo
parameter will be mapped tocustom.thing.foo
in the output,bar
will be mapped as-is. This enables rich parameter control which is useful for Events and any other usage of theLoggerMessage
attribute. If the name is remapped this way then it should be possible to use the remapped name as part of the Formatted message if desired, this gives developers more flexibility in how they name the parameters and have full control over the output format.AnyValue Definition
We could potentially defer the AnyValue support to the OTel library and have a type checker on what is passed in, but I think we should probably embrace AnyValue and enable its use in other places such as tags on Activity, that according to OTLP can have AnyValue members.
AnyValue should be added as a class to a
Microsoft.Extensions.*
package. It is probably better in the Extensions repo rather than runtime, as there is more schedule flexibility. This will then enable applications to use AnyValue typed arguments in logging, activity/spans, and event tags. The existing APIs all accept object parameters, which means that applications can pass in AnyValue.Having the explicit type will make it much easier for applications to ensure that they have formatted complex data in a way that OTel and other exporters can understand.
We should use the affinity to JSON to override the
AnyValue.ToString()
functionality to return JSON formatted data. In most logging providers, they will use theToString()
when they do not understand the type. This will allow applications to use AnyValue with legacy providers and still have a good experience.IAnyValue Interface
To facilitate easier passing of complex objects into APIs such as
Activity.Tags
or as tags on log messages, we should include a conversion interface that custom classes can implement. This is a marker that the type can be converted into an AnyValue and that its values can be translated to JSON.The implementer of the interface is responsible for the work. It will typically involve creating a Dictionary<string, AnyValue> populated with the data from the object.
The advantage of using this approach is that the actual conversion to the AnyValue structure can be delayed to the last minute, so if the telemetry is not emitted, the conversion work can be skipped,
ILogger TState
ILogger uses a struct
FormattedLogValues
to store the log message data. As events do not have a message, it's not always clear what should happen for the format provider function. We should probably default to one that will emit the Event data as a JSON blob - that way it will emit all of the data even with legacy log output providers etc. However, where those providers also emit each value, that becomes duplicative and expensive.We need a marker that the logging provider can use to know what to do. Today the special name
{OriginalFormat}
is used to mark the format string, but that can be a bit brittle (issues with the{}
being invalid in data sent to kusto for example). We should have a better way to know what to do.The text was updated successfully, but these errors were encountered: