search

open-telemetry / opentelemetry-go

OpenTelemetry Go API and SDK
https://opentelemetry.io/
Apache License 2.0
5.11k stars 1.03k forks source link

Introduce a functional option for `span.RecordError` to request that sets the status if the supplied `error` is not nil #1677

Open MrAlias opened 3 years ago

MrAlias commented 3 years ago

Originally posted by @seh in https://github.com/open-telemetry/opentelemetry-go/issues/1661#issuecomment-791762744

lastchiliarch commented 3 years ago

I'd like to take this one.

I'm going to add an IsRecordError filed to SpanConfig and new event option WithRecordError. If spanConfig.IsRecordError is true when RecordError fucntion is called with options inlucde WithRecordError, I will call the s.SetStatus(codes.Error, "") to set status.

Glad to hear from you, if anything was misunderstood.

MrAlias commented 3 years ago

I'm going to add an IsRecordError filed to SpanConfig and new event option WithRecordError. If spanConfig.IsRecordError is true when RecordError fucntion is called with options inlucde WithRecordError, I will call the s.SetStatus(codes.Error, "") to set status.

I'm not sure this is what I or others were thinking originally. Instead, add an EventOption that was named WithStatus. E.g.

func WithStatus() EventOption

This would be passed to the call of RecordError when instrumentation wanted to record an error and set the span status to Error.

treuherz commented 2 years ago

It looks like this got stuck after #1677 fell behind a bunch of refactors. If @lastchiliarch isn't able to take it forward I'd be interested in picking this up in a new PR.

tmc commented 5 days ago

@treuherz this would be nice to see in.

treuherz commented 4 days ago

Apologies, I wasn’t able to find a satisfactory backwards-compatible way of making this happen. Maybe inspiration will strike someone else!

amanakin commented 3 days ago

Hey @MrAlias, tried to solve an issue. Can you take a look at PR, please?