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

Move stack trace formatting out of Span class #632

Closed
tidal opened this issue Mar 25, 2022 · 3 comments · Fixed by #694
Closed

Move stack trace formatting out of Span class #632

tidal opened this issue Mar 25, 2022 · 3 comments · Fixed by #694
Assignees
Labels
bite sized This is a small chunk of work (good for new or time limited contributors!) good first issue Good for newcomers help wanted This issue is looking for someone to work on it refactoring Doesn't change behavior, but improves performance or maintainability or removes technical debt.

Comments

@tidal
Copy link
Member

tidal commented Mar 25, 2022

The method formatStackTrace should be moved from Span into a dedicated Util/Formatter class in the SDK\Common namespace since formatting stack traces is not the responsibility of a Span.

@tidal tidal added help wanted This issue is looking for someone to work on it good first issue Good for newcomers bite sized This is a small chunk of work (good for new or time limited contributors!) refactoring Doesn't change behavior, but improves performance or maintainability or removes technical debt. labels Mar 25, 2022
@amber0612
Copy link
Contributor

@tidal Please assign this issue to me. Thanks!

@amber0612
Copy link
Contributor

Inorder to test, i need to know how we can re-produce the scenario which uses formatStackTrace function as I cannot find code which is currently using this function

@brettmc
Copy link
Collaborator

brettmc commented Jun 1, 2022

Span::recordException() calls the static method formatStackTrace: https://github.com/open-telemetry/opentelemetry-php/blob/main/src/SDK/Trace/Span.php#L295
Usually it would be invoked by something like

try {
  //something that might throw
} catch (\Throwable $t) {
  $span->recordException($t);
}

@tidal tidal closed this as completed in #694 Jun 3, 2022
tidal pushed a commit that referenced this issue Jun 3, 2022
* Otel-php:632 Move stack trace formatting out of Span class

* added function usages

* fix linting errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bite sized This is a small chunk of work (good for new or time limited contributors!) good first issue Good for newcomers help wanted This issue is looking for someone to work on it refactoring Doesn't change behavior, but improves performance or maintainability or removes technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants