- 
                Notifications
    You must be signed in to change notification settings 
- Fork 536
Docs: WIP V2/6.5 additions and changes #1530
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
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.
Great improvements! 😃
| Thanks @watson! | 
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.
@bmorelli25 this is great work! Looking forward to get the docs updated.
The example docs in the transaction, error and span indices are not updated, also the examples for the Intake API are not updated. I created an issue for that #1532
The  apm-server.concurrent_requests setting is deprecated and only used in v1. It is mentioned a couple of times, in the common problems section and also in the general config settings  section. I think this should be marked as deprecated. I would not remove it, as it might be helpful to find for users that haven't upgraded.
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.
Fantastic work @bmorelli25 ! Other than some imprecisions mostly pointed by Silvia, this looks great.
You found sooo many typos 🙈
| @@ -0,0 +1,20 @@ | |||
| [[metricset-api]] | |||
| === Metricsets | |||
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.
Hmm are metrics included in the sections about event types? Should be?
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.
They are not, mostly because I'm not quite sure I fully understand they are yet.
| 
 I'd vote for removing that. Also with the new data model section, i think the overview could be shorted and higher level. | 
| Defines the host and port the server is listening on. | ||
| Use "unix:/path/to.sock" to listen on a unix domain socket. | ||
| Defaults to 'localhost:8200'. | ||
|  | 
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.
I realized there are a couple of config options missing in the Configuring APM Server chapter. Some of them were newly introduced, some not. I suggest to check with the apm-server.yml for missing config options (e.g.: apm-server.rum.event_rate and apm-server.rum.source_mapping.elasticsearch should be added to Set up Real User Monitoring (RUM) support.
For max_event_size you explicitly added that it was introduced with 6.5. On one hand it is nice to see newly introduced options, on the other hand it means we have to remove this again for 6.6 or add the info when an option was introduced everywhere.
For everything related to tuning and config options we need some strategy to point out what to tweak and which options to use when agents talk Intake v1 vs. agents talking intake v2, e.g. if somebody uses a ruby agent with intake v1 and a nodejs agent with intake v2, then both - the deprecated and the newly introduced config options - should be set properly.
Not sure how much you want to put into this PR, feel free to create a new issue and defer this.
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.
It looks like apm-server.rum.source_mapping.elasticsearch was already added, I've now added in event_rate as well.
configuration-process.asciidoc and configuration-rum.asciidoc have been updated. Changed the order of configuration options to better mirror apm-server.yml, and added in notes about v1/v2 to hopefully make things clearer.
The tuning is a bit more complex, so I've opened #1537
        
          
                docs/data-ingestion.asciidoc
              
                Outdated
          
        
      |  | ||
| [[adjust-concurrent-requests]] | ||
| [float] | ||
| === Adjust concurrent requests | 
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.
Sorry for not being more clear in my last review - adjusting concurrent request is only an option if you use agents talking Intake v1. I'd therefore add a deprecated warning to this whole section. But we also need to find a way to make it visible which adjustments help for agents using v1 and which for agents using v2 (see my comment above).
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.
Updated the tag for now, opened #1537 for later
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.
Great work @bmorelli25, thank you for all the effort!
| [options="header"] | ||
| |======================================================================= | ||
| |Agent Version |APM Server Version | ||
| |0.x |6.3-6.4 | 
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.
Can you also remove 0.x here - afterwards feel free to merge this PR.
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.
Before deciding to remove see #1530 (comment)
* docs: rework apm overview * docs: move events and add distributed tracing * docs: add v2 docs * Update example files for official docs. (#2) * docs: add redirects file * docs: add agent server compatibility
* docs: rework apm overview * docs: move events and add distributed tracing * docs: add v2 docs * Update example files for official docs. (#2) * docs: add redirects file * docs: add agent server compatibility
* docs: rework apm overview * docs: move events and add distributed tracing * docs: add v2 docs * Update example files for official docs. (#2) * docs: add redirects file * docs: add agent server compatibility
* docs: rework apm overview * docs: move events and add distributed tracing * docs: add v2 docs * Update example files for official docs. (#2) * docs: add redirects file * docs: add agent server compatibility
6.5 Server & Overview documentation.
Attempting to display V1/V2 information side by side in the docs was difficult and felt confusing and cluttered. Because of this, I decided to replace V1 information with V2, and link to an "upgrade guide" which explains the changes that were made.
Additions/Changes (some still in progress)
docs/guide/apm-data-model.asciidocNew APM Overview structure
Updates needed to agent documentation:
Redirects needed:
https://www.elastic.co/guide/en/apm/server/6.5/event-types.html
https://www.elastic.co/guide/en/apm/server/6.5/transactions.html
https://www.elastic.co/guide/en/apm/server/6.5/errors.html
https://www.elastic.co/guide/en/apm/get-started/6.5/apm-data-model.html
https://www.elastic.co/guide/en/apm/get-started/6.5/transactions.html
https://www.elastic.co/guide/en/apm/get-started/6.5/errors.html