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

Rename record_error to record_exception per spec #927

Merged
merged 4 commits into from
Jul 24, 2020

Conversation

codeboten
Copy link
Contributor

The spec renamed record_error to record_exception. This change follows that change.
@codeboten codeboten requested a review from a team July 20, 2020 21:32
@@ -683,14 +683,14 @@ def __exit__(

super().__exit__(exc_type, exc_val, exc_tb)

def record_error(self, err: Exception) -> None:
"""Records an error as a span event."""
def record_exception(self, exception: Exception) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe for another PR, but is passing in the type, message and stack trace as parameters also valid for this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the spec, the recommendation was to have a convenience method to record exceptions (https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#record-exception).

This seemed more pythonic than passing in exception parameters, but i'm happy to add another PR if you feel strongly about it.

@codeboten codeboten merged commit 935280c into open-telemetry:master Jul 24, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
…pen-telemetry#930)

* chore(metrics): move each aggregator in its own file

* feat(aggregators): implement histogram aggregator open-telemetry#927

* chore: address PR comments

* chore: fix ConsoleMetricExporter

* chore: address mayur comments

* chore: add documentation on internal structure
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.

3 participants