Skip to content

Update framework integration docs#689

Merged
bobstrecansky merged 7 commits into
open-telemetry:mainfrom
yuktea:update-integration-docs
Jun 2, 2022
Merged

Update framework integration docs#689
bobstrecansky merged 7 commits into
open-telemetry:mainfrom
yuktea:update-integration-docs

Conversation

@yuktea

@yuktea yuktea commented May 28, 2022

Copy link
Copy Markdown
Contributor

Updates Laravel and Symfony integration docs for consistency with API changes, correctness and completeness. Also fixes some language and fixes the md styling.

@codecov

codecov Bot commented May 28, 2022

Copy link
Copy Markdown

Codecov Report

Merging #689 (b056e95) into main (499c58a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #689   +/-   ##
=========================================
  Coverage     85.00%   85.00%           
  Complexity     1233     1233           
=========================================
  Files           137      137           
  Lines          2980     2980           
=========================================
  Hits           2533     2533           
  Misses          447      447           
Flag Coverage Δ
7.4 85.00% <ø> (ø)
8.0 85.05% <ø> (ø)
8.1 85.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 499c58a...b056e95. Read the comment docs.

Comment thread docs/laravel-integration.md
Comment thread docs/symfony-integration.md Outdated
Comment thread docs/symfony-integration.md Outdated
- Avoid multiple constructor calls.
- Grammar regression.
- Fix type `s/jaegar/jaeger`.
@yuktea

yuktea commented May 29, 2022

Copy link
Copy Markdown
Contributor Author

@bobstrecansky should be okay to merge?

Comment thread docs/symfony-integration.md Outdated
Comment thread docs/symfony-integration.md Outdated
Comment thread docs/symfony-integration.md Outdated
Comment thread docs/laravel-integration.md Outdated
@yuktea

yuktea commented May 30, 2022

Copy link
Copy Markdown
Contributor Author

@Nevay @brettmc I've updated the tracer/exporters with the new API usage and tried to improve things across the board including fixing the project setup instructions for Symfony and the cleaning up imports, updating instructions in both the docs. Please take a look

Comment thread docs/symfony-integration.md Outdated
@yuktea
yuktea force-pushed the update-integration-docs branch from bbec4c9 to ff384bb Compare May 30, 2022 13:48
Comment thread docs/laravel-integration.md Outdated
Comment thread docs/laravel-integration.md Outdated
throw new \Exception('Exception Example');
} catch (\Exception $exception) {
$span->setSpanStatus($exception->getCode(), $exception->getMessage());
$childSpan->setSpanStatus($exception->getCode(), $exception->getMessage());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Span::setStatus() accepts StatusCode::STATUS_* now, and this should now use Span::recordException. See SpanInterface

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we still set a status or only record the exception without marking the error status?

@yuktea yuktea mentioned this pull request May 31, 2022
@yuktea

yuktea commented May 31, 2022

Copy link
Copy Markdown
Contributor Author

With 8739706 I've demoted these examples to "exploration guides". This was done in consultation with @tidal.
I'm yet to rename the files, encourage sharing if someone has suggestions for new names.

@bobstrecansky

Copy link
Copy Markdown
Contributor

Perhaps a good naming convention would be:

laravel-quickstart.md
symfony-quickstart.md

@bobstrecansky
bobstrecansky merged commit 88db903 into open-telemetry:main Jun 2, 2022
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.

4 participants