Skip to content

api: Adding record_error to span API#790

Merged
toumorokoshi merged 24 commits intoopen-telemetry:masterfrom
codeboten:add_record_error
Jun 12, 2020
Merged

api: Adding record_error to span API#790
toumorokoshi merged 24 commits intoopen-telemetry:masterfrom
codeboten:add_record_error

Conversation

@codeboten
Copy link
Contributor

As per open-telemetry/opentelemetry-specification#427, we need an interface for users to record errors. This PR provides one possible solution to record the error as an event on the span.

@codeboten codeboten requested a review from a team June 8, 2020 22:01
Copy link
Contributor

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor suggestion

cnnradams and others added 21 commits June 11, 2020 13:26
The span context is no longer coupled with the tracer itself.
As such, providing a get_current_span method bound to the
trace api module rather than a specific tracer is semantically
correct, and removes a hurdle where someone who wants to retrieve
the current trace would have to create a tracer to do so.

renaming and exporting get_span_in_context to get_current_span,
as the intention of the API is similar, and reduces unneeded
aliasing and duplication.

set_span_in_context is not renamed, as set_current_span would have
implied that the span would then be active in the default context,
which is only true after attaching the resulting context returned
by set_span_in_context. Keeping that name at least implies some
asymmetric behavior from get_current_span.

After discussion in the SIG, we decided to remove the
legacy get_current_span APIs from Tracer and TracerProvider
to reduce long-term confusion of how to idiomatically retrieve
the span.

Co-authored-by: alrex <aboten@lightstep.com>
Co-authored-by: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>
Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>
Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io>
Co-authored-by: Andrew Xue <aaxue@google.com>
Co-authored-by: Cheng-Lung Sung <clsung@gmail.com>
Initial addition of opentelemetry-proto based protobufs.

Co-authored-by: alrex <alrex.boten@gmail.com>
Co-authored-by: Yusuke Tsutsumi <yusuke@tsutsumi.io>
Adding an exporter that supports sending metrics data to cloud monitoring (formerly known as stackdriver).

Co-authored-by: Chris Kleinknecht <libc@google.com>
Co-authored-by: Yusuke Tsutsumi <yusuke@tsutsumi.io>
Adding initial boto core implementation.

Co-authored-by: alrex <alrex.boten@gmail.com>
Co-authored-by: Yusuke Tsutsumi <yusuke@tsutsumi.io>
Co-authored-by: Yusuke Tsutsumi <yusuke@tsutsumi.io>
eachdist.py did not support the installation of test packages,
(as defined by the extra_requires:test package group). As a
result, test packages were being added to dev-requirements.txt

By having eachdist.py develop install test packages, and moving
develop/test package definitions to the individual instrumentations,
it is easier to determine which packages require which dependencies
for testing purposes, and enables support for existing dependencies
that follow the extra_requires:test pattern.
Co-authored-by: Leighton Chen <lechen@microsoft.com>
Co-authored-by: alrex <aboten@lightstep.com>
opentelemetry-test was not listed as a test dependency in the asgi instrumentation.
Minor cleanup of setup.cfg files. Updated the example app to move configuration from setup.py into setup.cfg as well.

Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>
Co-authored-by: Yusuke Tsutsumi <yusuke@tsutsumi.io>
adding some troubleshooting tips for users with common failures
Copy link
Member

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@toumorokoshi toumorokoshi merged commit d22ed88 into open-telemetry:master Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants