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

PHP: Updates to instrumentation docs #3855

Merged
merged 6 commits into from
Jan 30, 2024

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented Jan 24, 2024

Contributes to #3229, this updates the tracing instructions to follow a similar path what we have for some other languages already (JS was the example I took for reference). Note that my PHP knowledge is a little bit outdated, so if there are better ways to inject tracers etc advice me accordingly.

@svrnm svrnm added the sig:php label Jan 24, 2024
@svrnm svrnm requested review from a team January 24, 2024 16:26
content/en/docs/languages/php/instrumentation.md Outdated Show resolved Hide resolved
content/en/docs/languages/php/instrumentation.md Outdated Show resolved Hide resolved
content/en/docs/languages/php/instrumentation.md Outdated Show resolved Hide resolved
Comment on lines 37 to 39
To highlight the difference between instrumenting a _library_ and a standalone
_app_, split out the dice rolling into a _library file_, which then will be
imported as a dependency by the _app file_.
Copy link
Member

Choose a reason for hiding this comment

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

I'd remove the italics here. Not needed.

_app_, split out the dice rolling into a _library file_, which then will be
imported as a dependency by the _app file_.

Create the _library file_ named `dice.php` and add the following code to it:
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.


The example also sets up the mandatory SDK default attribute `service.name`,
which holds the logical name of the service, and the optional (but highly
encouraged!) attribute `service.version`, which holds the version of the service
Copy link
Member

Choose a reason for hiding this comment

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

I think I've left similar comments in #3834 — we should sweep the docs everywhere to fix the issues...

[`Tracer`](/docs/concepts/signals/traces/#tracer).
If a `TracerProvider` is not created, the OpenTelemetry APIs for tracing will
use a no-op implementation and fail to generate data. As explained next, modify
the `instrumentation.ts` (or `instrumentation.js`) file to include all the SDK
Copy link
Contributor

Choose a reason for hiding this comment

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

js/node/browser references here. The filename references probably need index.php...

Copy link
Member Author

Choose a reason for hiding this comment

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

oh... let me double check on that

Co-authored-by: Fabrizio Ferri-Benedetti <[email protected]>
Co-authored-by: Brett McBride <[email protected]>
@svrnm svrnm merged commit cbec47d into open-telemetry:main Jan 30, 2024
14 checks passed
@svrnm svrnm deleted the php-instrumentations-update branch January 30, 2024 14:13
jaydeluca pushed a commit to jaydeluca/opentelemetry.io that referenced this pull request Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants