Skip to content

api: Adding record_error to span API #790

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

Merged
merged 24 commits into from
Jun 12, 2020

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 <[email protected]>
Co-authored-by: Hector Hernandez <[email protected]>
Co-authored-by: Leighton Chen <[email protected]>
Co-authored-by: Diego Hurtado <[email protected]>
Co-authored-by: Mauricio Vásquez <[email protected]>
Co-authored-by: Andrew Xue <[email protected]>
Co-authored-by: Cheng-Lung Sung <[email protected]>
Initial addition of opentelemetry-proto based protobufs.

Co-authored-by: alrex <[email protected]>
Co-authored-by: Yusuke Tsutsumi <[email protected]>
Adding an exporter that supports sending metrics data to cloud monitoring (formerly known as stackdriver).

Co-authored-by: Chris Kleinknecht <[email protected]>
Co-authored-by: Yusuke Tsutsumi <[email protected]>
Adding initial boto core implementation.

Co-authored-by: alrex <[email protected]>
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.
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 <[email protected]>
Co-authored-by: Yusuke Tsutsumi <[email protected]>
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