open-telemetry / opentelemetry-go

OpenTelemetry Go API and SDK
https://opentelemetry.io/docs/languages/go
Apache License 2.0
5.25k stars 1.07k forks source link

Log KeyValue Kind: KindError #5909

Open Jesse0Michael opened 2 days ago

Jesse0Michael commented 2 days ago

Problem Statement

I am trying to write a custom Log Record Processor that will look at the record attributes for a KeyValue containing an error, and, if the record severity >= Error and it found an error Value that matches criteria included when configuring the processor, downgrade the severity to a Warn.

This is something I've done in other logging solutions with hooks or handlers. But I'm unable to write an otel Log Processor because there is no way to pull the error that was used as a log attribute, because the error value is converted to a string. example: If the error is a context.Canceled, downgrade the severity of the log to a Warn

Proposed Solution

Would it be alright to add a new KindError to the log KeyValue Kinds

With supported func Error(key string, value error) KeyValue and func ErrorValue(v error) Value and func (v Value) AsError() error functions.

Jesse0Michael commented 2 days ago

If this is acceptable approach, I could work on the PR for it (as well as the subsequent PRs in https://github.com/open-telemetry/opentelemetry-go-contrib that would want to be updated to use the new Kind)

dmathieu commented 2 days ago

I feel like this would break the specification requirements: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#type-any

@pellared what do you think about this proposal?

pellared commented 1 day ago

@Jesse0Michael, first of all, thanks for raising the issue.

I agree with @dmathieu that this would be not compliant with the specification. The SDK is designed to support the OpenTelemetry data-model and be compliant with the specification (see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#additional-logrecord-interfaces). Therefore, it does not have kinds for all the core/common Go types. We could say that we also lack KindTime, KindDuration, KindComplex if we would like to support more Go core types.

If you think that the current design is not good (e.g. not user friendly), feel free to create an issue in https://github.com/open-telemetry/opentelemetry-specification so we can discuss it further.

This is something I've done in other logging solutions with hooks or handlers.

I think currently this is the way to go... E.g. decorate https://pkg.go.dev/go.opentelemetry.io/contrib/bridges/otelslog.