-
Notifications
You must be signed in to change notification settings - Fork 230
docs: update broken links for apm-guide-ref #1385 #2541
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
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Olly! LG. I'll tag the Node.js team for review and create the lone backport.
@@ -373,7 +373,7 @@ still supported. Users should switch to using `longFieldMaxLength`. If | |||
now use the `longFieldMaxLength` value. | |||
+ | |||
Note that ultimately the maximum length of any tracing field is limited by the | |||
{apm-server-ref-v}/configuration-process.html#max_event_size[`max_event_size`] | |||
{apm-guide-ref}/configuration-process.html#max_event_size[`max_event_size`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elastic/observability-docs#1385 suggests this one should be "Remove from Node.js Agent"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was an incorrect assumption on my part. This config variable is still valid for the standalone method of running APM Server. We'll need to remove this at a later date.
@@ -104,7 +104,7 @@ containing the data about to be sent to the APM Server. | |||
|
|||
The format of the payload depends on the event type being sent. | |||
For details about the different formats, | |||
see the {apm-server-ref-v}/intake-api.html[APM Server intake API documentation]. | |||
see the {apm-guide-ref}/intake-api.html[APM Server intake API documentation]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elastic/observability-docs#1385 says
{apm-server-ref}/intake-api.html
-> {apm-guide-ref}/api.html
Though perhaps for this one we'd (eventually?) want https://www.elastic.co/guide/en/apm/guide/7.17/api-events.html ? I'm not sure if the intent of this PR is to just fix the {apm-server-ref-v}
reference, or to also change from the "Legacy APM Server Reference" to the new Agent-based APM server docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, the goal here is just to fix the outdated {apm-server-ref-v}
refs. There are improvements that could be made to all of the agent docs to better differentiate between running APM Server in standalone mode vs the APM integration, but I don't have the bandwidth to tackle those right now.
As for the API link. It looks like this was a mistake, but I'd agree that the current link is better than what I had proposed in elastic/observability-docs#1385.
Links updated as per elastic/observability-docs#1385. The old links will break when 7.17 releases. This PR and all backports must be merged before the 7.17 release. Thanks!
Backports