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.

Sign up for GitHub

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

Jump to bottom

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
Open

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

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)
@MrAlias MrAlias mentioned this issue Mar 8, 2021
Closed
@MrAlias MrAlias added area:trace Part of OpenTelemetry tracing pkg:API Related to an API package release:after-ga labels Mar 8, 2021
@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
Add WithStatus option to set Span status at the same time (open-telem…
74fa4b6 
…etry#1677)

Signed-off-by: lastchiliarch <lastchiliarch@163.com>
@lastchiliarch lastchiliarch mentioned this issue Apr 17, 2021
Closed
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
@lastchiliarch
Add WithStatus option to set Span status at the same time (open-telem…
5d238e8 
…etry#1677)

Signed-off-by: lastchiliarch <lastchiliarch@163.com>
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
@lastchiliarch
Add WithStatus option to set Span status at the same time (open-telem…
5ae2893 
…etry#1677)

Signed-off-by: lastchiliarch <lastchiliarch@163.com>
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
@lastchiliarch
Add WithStatus option to set Span status at the same time (open-telem…
1d42dfa 
…etry#1677)

Signed-off-by: lastchiliarch <lastchiliarch@163.com>
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
@lastchiliarch
Add WithStatus option to set Span status at the same time (open-telem…
d3b58b6 
…etry#1677)

Signed-off-by: lastchiliarch <lastchiliarch@163.com>
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
@lastchiliarch
Add WithStatus option to set Span status at the same time (open-telem…
c1573c2 
…etry#1677)

Signed-off-by: lastchiliarch <lastchiliarch@163.com>
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 17, 2021
@lastchiliarch
Add WithStatus option to set Span status at the same time (open-telem…
b1ee5d9 
…etry#1677)
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue Apr 27, 2021
@lastchiliarch
Changed signature of the Span RecordError, replace EventOption by E…
364ad1e 
…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 <lastchiliarch@163.com>
lastchiliarch added a commit to lastchiliarch/opentelemetry-go that referenced this issue May 5, 2021
@lastchiliarch
Changed signature of the Span RecordError, replace EventOption by E…
92558f9 
…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 <lastchiliarch@163.com>
@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 treuherz mentioned this issue May 5, 2022
Closed
treuherz added a commit to treuherz/opentelemetry-go that referenced this issue May 5, 2022
@treuherz
Add WithStatus option for use with RecordError
7b605f0 
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
@treuherz
Add WithStatus option for use with RecordError
201ed00 
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 amanakin linked a pull request Sep 2, 2024 that will close this issue
Open
@amanakin
Copy link
Contributor

amanakin commented Sep 3, 2024

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

pellared added a commit that referenced this issue Mar 26, 2025
@MrAlias @pellared
Generate v1.31.0 semconv (#6479)
95e5bba 
- Generate `semconv/v1.31.0`
- Stop generating deprecated metric semconv similar to all other
generation
- Fix acronyms:
  - `ReplicationController`
  - `ResourceQuota`

## [`v1.31.0` semantic convention release
notes](https://github.com/open-telemetry/semantic-conventions/releases/tag/v1.31.0):

<h3>🛑 Breaking changes 🛑</h3>
<ul>
<li>
<p><code>code</code>: <code>code.function.name</code> value should
contain the fully qualified function name, <code>code.namespace</code>
is now deprecated (<a
href="open-telemetry/semantic-conventions#1677"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1677/hovercard">#1677</a>)</p>
</li>
<li>
<p><code>gen-ai</code>: Introduce <code>gen_ai.output.type</code>and
deprecate <code>gen_ai.openai.request.response_format</code> (<a
href="open-telemetry/semantic-conventions#1757"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1757/hovercard">#1757</a>)</p>
</li>
<li>
<p><code>mobile</code>: Rework <code>device.app.lifecycle</code> mobile
event. (<a
href="open-telemetry/semantic-conventions#1880"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1880/hovercard">#1880</a>)<br>
The <code>device.app.lifecycle</code> event has been reworked to use
attributes instead<br>
of event body fields. The <code>ios.app.state</code> and
<code>android.app.state</code> attributes<br>
have been reintroduced to the attribute registry.</p>
</li>
<li>
<p><code>system</code>: Move CPU-related system.cpu.* metrics to CPU
namespace (<a
href="open-telemetry/semantic-conventions#1873"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1873/hovercard">#1873</a>)</p>
</li>
<li>
<p><code>k8s</code>: Change k8s.replication_controller metrics to
k8s.replicationcontroller (<a
href="open-telemetry/semantic-conventions#1848"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1848/hovercard">#1848</a>)</p>
</li>
<li>
<p><code>db</code>: Rename <code>db.system</code> to
<code>db.system.name</code> in database metrics, and update the values
to be consistent with database spans. (<a
href="open-telemetry/semantic-conventions#1581"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1581/hovercard">#1581</a>)</p>
</li>
<li>
<p><code>session</code>: Move <code>session.id</code> and
<code>session.previous_id</code> from body fields to event attributes,
and yamlize <code>session.start</code> and <code>session.end</code>
events. (<a
href="open-telemetry/semantic-conventions#1845"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1845/hovercard">#1845</a>)<br>
As part of the ongoing migration of event fields from LogRecord body to
extended/complex attributes, the <code>session.start</code> and
<code>session.end</code> events have been redefined.</p>
</li>
</ul>
<h3>💡 Enhancements 💡</h3>
<ul>
<li>
<p><code>code</code>: Mark <code>code.*</code> semantic conventions as
release candidate (<a
href="open-telemetry/semantic-conventions#1377"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1377/hovercard">#1377</a>)</p>
</li>
<li>
<p><code>gen-ai</code>: Added AI Agent Semantic Convention (<a
href="open-telemetry/semantic-conventions#1732"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1732/hovercard">#1732</a>,
<a
href="open-telemetry/semantic-conventions#1739"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1739/hovercard">#1739</a>)</p>
</li>
<li>
<p><code>db</code>: Add database-specific notes on db.operation.name and
db.collection.name for Cassandra, Cosmos DB, HBase, MongoDB, and Redis,
covering their batch/bulk terms and lack of cross-table queries. (<a
href="open-telemetry/semantic-conventions#1863"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1863/hovercard">#1863</a>,
<a
href="open-telemetry/semantic-conventions#1573"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1573/hovercard">#1573</a>)</p>
</li>
<li>
<p><code>gen-ai</code>: Adds <code>gen_ai.request.choice.count</code>
span attribute (<a
href="open-telemetry/semantic-conventions#1888"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1888/hovercard">#1888</a>)<br>
Enables recording target number of completions to generate</p>
</li>
<li>
<p><code>enduser</code>: Undeprecate 'enduser.id' and introduce new
attribute <code>enduser.pseudo.id</code> (<a
href="open-telemetry/semantic-conventions#1104"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1104/hovercard">#1104</a>)<br>
The new attribute <code>enduser.pseudo.id</code> is intended to provide
a unique identifier of a pseudonymous enduser.</p>
</li>
<li>
<p><code>k8s</code>: Add <code>k8s.hpa</code>,
<code>k8s.resourcequota</code> and
<code>k8s.replicationcontroller</code> attributes and resources (<a
href="open-telemetry/semantic-conventions#1656"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1656/hovercard">#1656</a>)</p>
</li>
<li>
<p><code>k8s</code>: How to populate resource attributes based on
attributes, labels and transformation (<a
href="open-telemetry/semantic-conventions#236"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/236/hovercard">#236</a>)</p>
</li>
<li>
<p><code>process</code>: Adjust the semantic expectations for
<code>process.executable.name</code> (<a
href="open-telemetry/semantic-conventions#1736"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1736/hovercard">#1736</a>)</p>
</li>
<li>
<p><code>otel</code>: Adds SDK self-monitoring metrics for span
processing (<a
href="open-telemetry/semantic-conventions#1631"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1631/hovercard">#1631</a>)</p>
</li>
<li>
<p><code>cicd</code>: Adds a new attribute
<code>cicd.pipeline.run.url.full</code> and corrects the attribute
description of <code>cicd.pipeline.task.run.url.full</code> (<a
href="open-telemetry/semantic-conventions#1796"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1796/hovercard">#1796</a>)</p>
</li>
<li>
<p><code>user-agent</code>: Add <code>user_agent.os.name</code> and
<code>user_agent.os.version</code> attributes (<a
href="open-telemetry/semantic-conventions#1433"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1433/hovercard">#1433</a>)</p>
</li>
</ul>
<h3>🧰 Bug fixes 🧰</h3>
<ul>
<li><code>process</code>: Fix units of
process.open_file_descriptor.count and process.context_switches (<a
href="open-telemetry/semantic-conventions#1662"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1662/hovercard">#1662</a>)</li>
</ul>

---------

Co-authored-by: Robert Pająk <pellared@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:trace Part of OpenTelemetry tracing help wanted Extra attention is needed pkg:API Related to an API package
Projects
None yet
Milestone
No milestone
5 participants
@tmc @treuherz @lastchiliarch @MrAlias @amanakin