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

Add content to APM User guide #6378

Merged
merged 25 commits into from
Oct 29, 2021
Merged

Conversation

bmorelli25
Copy link
Member

@bmorelli25 bmorelli25 commented Oct 18, 2021

Summary

This PR adds the following content to the APM Guide:

  • Adds and updates the Source map upload guide to use Kibana APIs instead of APM Server APIs
  • Placeholder topics for ILM, Data streams, and Ingest pipelines. These are placeholder topics for now as the majority of the conceptual and reference content will live in the Fleet and Elastic Agent User Guide. An example will be added to each.
  • Updates and adds the Manage storage guide. I ran into some trouble with the Elasticsearch API examples. Please see the two comments below.
  • Updated Jaeger integration docs to point to EA instead of APM Server.
  • Updates the 7.14 input settings to include all new options.
  • Adds a placeholder page for upgrade instructions. This was needed to fix broken inbound links to the docs.
  • Moves the remaining legacy content (tab-widgets) to the /docs/legacy directory.

Related issues

For elastic/observability-docs#1073.

PRs blocked by this PR

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Oct 18, 2021
@mergify mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Oct 18, 2021
@bmorelli25 bmorelli25 self-assigned this Oct 18, 2021
@elastic elastic deleted a comment from mergify bot Oct 18, 2021
@apmmachine
Copy link
Contributor

apmmachine commented Oct 18, 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 preview

Expand to view the summary

Build stats

  • Start Time: 2021-10-29T19:57:06.387+0000

  • Duration: 41 min 58 sec

  • Commit: a9f7eb3

Test stats 🧪

Test Results
Failed 0
Passed 6422
Skipped 19
Total 6441

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /hey-apm : Run the hey-apm benchmark.

  • /package : Generate and publish the docker images.

@marclop marclop added backport-7.16 Automated backport with mergify to the 7.16 branch and removed backport-7.x labels Oct 20, 2021
Comment on lines 226 to 239
POST *-apm-*/_update_by_query
{
"query": {
"term": {
"service.name": {
"value": "current-service-name"
}
}
},
"script": {
"source": "ctx._source.service.name = 'new-service-name'",
"lang": "painless"
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm struggling to update this query. The _search works correctly, for example:

GET *-apm-*/_search
{
  "query": {
    "term": {
      "service.name": {
        "value": "opbeans-dotnet"
      }
    }
  }
}

But running the script returns 0 updates.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why this is necessary, but try adding ?expand_wildcards=all. I thought the default would match it... maybe a bug?

Copy link
Member Author

Choose a reason for hiding this comment

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

That did it!

Screen Shot 2021-10-29 at 12 37 03 PM

I don't think it's a bug. Reading into expand_wildcards, I can see that my APM indices are "hidden indices" so it makes sense that all would be needed to match a hidden index.

Screen Shot 2021-10-29 at 12 39 30 PM

@bmorelli25 bmorelli25 requested a review from a team October 25, 2021 22:08
@bmorelli25 bmorelli25 marked this pull request as ready for review October 25, 2021 22:08
@elastic elastic deleted a comment from mergify bot Oct 27, 2021
@bmorelli25 bmorelli25 added backport-8.0 Automated backport with mergify docs labels Oct 27, 2021
@bmorelli25
Copy link
Member Author

@elastic/apm-server Unfortunately, this PR is blocking me. If it's alright with y'all, I'm going to merge it tomorrow morning (PDT) so I can keep moving forward with docs updates. I know you're all super busy right now, so we can always review later and make updates in a future PR 😄

@axw
Copy link
Member

axw commented Oct 28, 2021

@bmorelli25 sorry, please go ahead and merge and we can do a post-merge review.

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Looks good!

[[view-edit-default-pipelines]]
=== View or edit a default pipeline

To view or edit a default pipelines in {kib},
Copy link
Member

Choose a reason for hiding this comment

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

Re "edit", we should make sure to document that the Fleet-managed ingest policy will be overwritten on reinstall/upgrade. Users can define their own custom ingest pipeline if needed, but they need make sure that they also call the package-provided pipeline.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me know if I'm understanding this correctly:

  1. Fleet adds ingest pipelines for the APM integration using index templates that include pipeline index settings.
  2. Elasticsearch uses the index pattern in these index templates to match pipelines to APM data streams.

As an example, the index template traces-apm, matches traces-apm-* data streams and assigns the "traces-apm-7.16.0" pipeline:

Screen Shot 2021-10-29 at 1 15 07 PM

  1. This, Fleet managed, index template ⬆️ will be overwritten on reinstall/upgrade.

Assuming the above is correct, then I see three scenarios:

  1. If a user edits the Fleet managed index template to point to a new pipeline, they need to:
    a. ensure the package-provided pipeline is also called
    b. know that the index template will be overwritten on install/upgrade

  2. If a user creates a custom component template with a higher priority that points to a new pipeline, they need to:
    a. ensure the package-provided pipeline is also called
    b. not sure what happens on upgrade here

  3. If a user edits the pipeline directly, like the "traces-apm-7.16.0" pipeline, they need to:
    a. ensure the package-provided pipelines are still included
    b. know that their edits will be overwritten on install/upgrade

Copy link
Member

Choose a reason for hiding this comment

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

I had thought that the default pipeline was defined in the @settings component template, which would allow it to be overridden. That is not the case, so #2 isn't possible actually. I believe at the moment users would be limited to #1 and #3. We'll need to improve this. CC @simitt

Users shouldn't need to care about upgrades, but rather they should have some way of hooking their own custom pipeline into a predefined location in the Fleet-managed ingest pipeline.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect. Thanks, Andrew!

Copy link
Contributor

Choose a reason for hiding this comment

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

started elastic/package-spec#129 a while back; @ruflin do you have any concerns when moving the default_pipeline to the component-template instead of the index-template to make it user overridable?

Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

Left some comments, but feel free to merge as is and address in a follow up. Apologies for such a long review time.

docs/apm-input-settings.asciidoc Show resolved Hide resolved
docs/source-map-how-to.asciidoc Outdated Show resolved Hide resolved
docs/source-map-how-to.asciidoc Show resolved Hide resolved
docs/ilm-how-to.asciidoc Show resolved Hide resolved
@bmorelli25 bmorelli25 merged commit d9a7812 into elastic:master Oct 29, 2021
@bmorelli25 bmorelli25 deleted the add-more-content branch October 29, 2021 20:38
mergify bot pushed a commit that referenced this pull request Oct 29, 2021
mergify bot pushed a commit that referenced this pull request Oct 29, 2021
@bmorelli25
Copy link
Member Author

bmorelli25 commented Oct 29, 2021

Thanks, @axw and @simitt! I've hit the merge button, but there are still a few good conversations and questions to address in this PR thread.

bmorelli25 added a commit that referenced this pull request Oct 29, 2021
(cherry picked from commit d9a7812)

Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
bmorelli25 added a commit that referenced this pull request Oct 29, 2021
(cherry picked from commit d9a7812)

Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-7.16 Automated backport with mergify to the 7.16 branch backport-8.0 Automated backport with mergify docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants