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

docs: Add PHP agent information to shared docs #4740

Merged
merged 12 commits into from
Feb 19, 2021

Conversation

bmorelli25
Copy link
Member

@bmorelli25 bmorelli25 commented Feb 16, 2021

Summary

This PR adds PHP agent information to the following documentation pages:

Observability Guide
APM Overview
APM Server Reference

Related issues

For elastic/observability-docs#383.

Blocked by elastic/apm-agent-php#352. Docs ci will not pass until 352 is merged.

@bmorelli25
Copy link
Member Author

@SergeyKleyman, please see the questions in the description above.

@apmmachine
Copy link
Contributor

apmmachine commented Feb 16, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #4740 updated

  • Start Time: 2021-02-19T17:44:20.789+0000

  • Duration: 46 min 27 sec

  • Commit: d538023

Test stats 🧪

Test Results
Failed 0
Passed 4751
Skipped 124
Total 4875

Trends 🧪

Image of Build Times

Image of Tests

Steps errors 2

Expand to view the steps failures

Run Window tests
  • Took 10 min 0 sec . View more details on here
Test Sync
  • Took 3 min 28 sec . View more details on here
  • Description: ./.ci/scripts/sync.sh

@SergeyKleyman
Copy link
Contributor

No OpenTracing bridge. Will there ever be one?

OpenTracing bridge is definitely not planned for PHP Agent 1.0 and I'm not sure if it will ever be added especially considering that OpenTracing is superseded by OpenTelemetry. If we get requests from users we will reconsider it.

What is the minimum supported APM Server version for 1.0?

For PHP Agent 1.0 minimum supported APM Server version is 7.0.

@bmorelli25
Copy link
Member Author

@elasticmachine, run elasticsearch-ci/docs

@bmorelli25 bmorelli25 marked this pull request as ready for review February 18, 2021 18:39
@codecov-io
Copy link

codecov-io commented Feb 18, 2021

Codecov Report

Merging #4740 (d538023) into master (ce812f6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4740   +/-   ##
=======================================
  Coverage   76.81%   76.81%           
=======================================
  Files         166      166           
  Lines       10244    10244           
=======================================
  Hits         7869     7869           
  Misses       2375     2375           
Impacted Files Coverage Δ
...ack/apm-server/aggregation/txmetrics/aggregator.go 93.36% <0.00%> (ø)

@@ -53,6 +53,7 @@ To configure the number of spans recorded per transaction, see the relevant Agen
* Java: {apm-java-ref-v}/config-core.html#config-transaction-max-spans[`transaction_max_spans`]
* .NET: {apm-dotnet-ref-v}/config-core.html#config-transaction-max-spans[`TransactionMaxSpans`]
* Node.js: {apm-node-ref-v}/configuration.html#transaction-max-spans[`transactionMaxSpans`]
* PHP: {apm-php-ref}/configuration-reference.html#config-transaction-max-spans[`transaction_max_spans`]
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the difference between apm-<language>-ref and apm-<language>-ref-v?

Copy link
Member Author

@bmorelli25 bmorelli25 Feb 18, 2021

Choose a reason for hiding this comment

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

Great catch! You're right, I should be using the -v attribute, which stands for version. The apm-<language>-ref resolves from here. It permanently links to current.

The apm-<language>-ref-v resolves from here, using the additional attributes described here. It points to the current version of the documentation as well, but allows us to pin agent versions to stack versions. This makes updating links as documentation evolves much easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in bc51362.

docs/guide/apm-data-model.asciidoc Outdated Show resolved Hide resolved
docs/tab-widgets/distributed-trace-receive.asciidoc Outdated Show resolved Hide resolved
@bmorelli25 bmorelli25 requested a review from a team February 18, 2021 23:59
docs/version.asciidoc Outdated Show resolved Hide resolved
@bmorelli25
Copy link
Member Author

Thank you @SergeyKleyman! I'll wait for one of the @elastic/obs-docs writers to give final approval before merging this PR.


2. Begin a new transaction using the agent's public API. For example, use {apm-php-ref-v}/public-api.html#api-elasticapm-class-begin-current-transaction[`ElasticApm::beginCurrentTransaction`]
and pass the new `DistributedTracingData` object as a parameter.
This will create a new transaction or span as a child of the incoming trace context.
Copy link
Contributor

@SergeyKleyman SergeyKleyman Feb 19, 2021

Choose a reason for hiding this comment

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

It's not entirely correct - as name implies beginCurrentTransaction is used to create a new transaction and not a span. I saw some other agents say something similar in this document (i.e., that distributed tracing context on the receiving side can be used to create a new span and not only a new transaction). I am not sure if it's correct for those agents or not - it might be worth making sure with the relevant teams. But for PHP agent distributed tracing context on the receiving side can be passed to create a new transaction but not a new span. A new spans can then be created as children of the new transaction (and the distributed tracing context will passed to them automatically by the new transaction). The confusion might be stemming from the fact that on the sending side the source of the distributed tracing context (that then is passed to the receiving side) can be either transaction or span whatever is the current entity at the point getDistributedTracingData is invoked. That source of the distributed tracing context will then be considered to be the parent of the new transaction on the receiving side.

Copy link
Member Author

@bmorelli25 bmorelli25 Feb 19, 2021

Choose a reason for hiding this comment

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

I am not sure if it's correct for those agents or not - it might be worth making sure with the relevant teams.

Thanks. The agent devs created that content for the most part, but I'll double-check just to be safe. I presume you may be correct and that we can do better with the wording here. I'll save that for a follow-up PR.

The confusion might be stemming from the fact that on the sending side the source of the distributed tracing context (that then is passed to the receiving side) can be either transaction or span whatever is the current entity at the point getDistributedTracingData is invoked.

Yes, that's very helpful. I'll fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in c9b17b5. Will follow up with the other agents in a separate PR.

@bmorelli25 bmorelli25 merged commit 2ce65df into elastic:master Feb 19, 2021
@bmorelli25 bmorelli25 deleted the add-php-agent-instructions branch February 19, 2021 18:53
bmorelli25 added a commit to bmorelli25/apm-server that referenced this pull request Feb 19, 2021
bmorelli25 added a commit to bmorelli25/apm-server that referenced this pull request Feb 19, 2021
bmorelli25 added a commit that referenced this pull request Feb 19, 2021
Co-authored-by: Sergey Kleyman <[email protected]>

Co-authored-by: Sergey Kleyman <[email protected]>
bmorelli25 added a commit that referenced this pull request Feb 19, 2021
Co-authored-by: Sergey Kleyman <[email protected]>

Co-authored-by: Sergey Kleyman <[email protected]>
v1v added a commit to v1v/apm-server that referenced this pull request Feb 22, 2021
…chemas-to-agents

* upstream/master: (111 commits)
  Introduce metricset.name (elastic#4857)
  processor/otel: test service.version handling (elastic#4853)
  docs: Add PHP agent information to shared docs (elastic#4740)
  Script for faster development workflow (elastic#4731)
  Update to elastic/beats@1b31c26 (elastic#4763)
  backport: add 7.12 to .backportrc.json (elastic#4807)
  backport: enable auto-merge on backport PRs (elastic#4777)
  Support for Node.js profiles (elastic#4728)
  docs: readds .NET link (elastic#4764)
  [DOCS] Fixes URLs on Secure communication with APM Agents page (elastic#4746)
  ci: set proper parameters for the tar step (elastic#4696)
  docs: add 7.11.1 release notes (elastic#4727)
  Disable sourcemap upload endpoint when data streams enabled (elastic#4735)
  Add service name to dataset field (elastic#4674)
  Update to elastic/beats@ba423212a660 (elastic#4733)
  sampling: require a default policy (elastic#4729)
  processor/otel: add unit test for span status (elastic#4734)
  Add support for consuming OTLP/gRPC metrics (elastic#4722)
  [apmpackage] Add config options supported in ESS (elastic#4690)
  Use the apm-server version everywhere* (elastic#4725)
  ...
v1v added a commit to v1v/apm-server that referenced this pull request Feb 22, 2021
…te-schema-json-1

* upstream/master: (111 commits)
  Introduce metricset.name (elastic#4857)
  processor/otel: test service.version handling (elastic#4853)
  docs: Add PHP agent information to shared docs (elastic#4740)
  Script for faster development workflow (elastic#4731)
  Update to elastic/beats@1b31c26 (elastic#4763)
  backport: add 7.12 to .backportrc.json (elastic#4807)
  backport: enable auto-merge on backport PRs (elastic#4777)
  Support for Node.js profiles (elastic#4728)
  docs: readds .NET link (elastic#4764)
  [DOCS] Fixes URLs on Secure communication with APM Agents page (elastic#4746)
  ci: set proper parameters for the tar step (elastic#4696)
  docs: add 7.11.1 release notes (elastic#4727)
  Disable sourcemap upload endpoint when data streams enabled (elastic#4735)
  Add service name to dataset field (elastic#4674)
  Update to elastic/beats@ba423212a660 (elastic#4733)
  sampling: require a default policy (elastic#4729)
  processor/otel: add unit test for span status (elastic#4734)
  Add support for consuming OTLP/gRPC metrics (elastic#4722)
  [apmpackage] Add config options supported in ESS (elastic#4690)
  Use the apm-server version everywhere* (elastic#4725)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants