Skip to content
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

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

Open
MrAlias opened this issue Mar 8, 2021 · 6 comments · May be fixed by #5762
Labels
area:trace Part of OpenTelemetry tracing help wanted Extra attention is needed pkg:API Related to an API package

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Mar 8, 2021

Originally posted by @seh in #1661 (comment)

@lastchiliarch
Copy link
Contributor

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
Copy link
Contributor Author

MrAlias commented Apr 16, 2021

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.

lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 27, 2021
…rrorOption (open-telemetry#1677)

* Change signature of the Span `RecordError, replace EventOption by ErrorOption
* Add WithEventOpts, WithErrorStatus
* Set status when WithErrorStatus is passed to RecordError

Signed-off-by: lastchiliarch <[email protected]>
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue May 5, 2021
…rrorOption (open-telemetry#1677)

* Change signature of the Span `RecordError, replace EventOption by ErrorOption
* Add WithEventOpts, WithErrorStatus
* Set status when WithErrorStatus is passed to RecordError

Signed-off-by: lastchiliarch <[email protected]>
@treuherz
Copy link

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.

@MrAlias MrAlias added help wanted Extra attention is needed and removed release:after-1.0 labels May 3, 2022
treuherz added a commit to treuherz/opentelemetry-go that referenced this issue May 5, 2022
Fixes open-telemetry#1677 by adding a functional option that sets the Status of a
span when calling RecordError.
treuherz added a commit to treuherz/opentelemetry-go that referenced this issue May 5, 2022
Fixes open-telemetry#1677 by adding a functional option that sets the Status of a
span when calling RecordError.
@tmc
Copy link

tmc commented Aug 31, 2024

@treuherz this would be nice to see in.

@treuherz
Copy link

treuherz commented Sep 1, 2024

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

@amanakin
Copy link
Contributor

amanakin commented Sep 3, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing help wanted Extra attention is needed pkg:API Related to an API package
Projects
None yet
5 participants