Skip to content

apm: Document sampling.tail.discard_on_write_failure config #1453

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jul 28, 2025
Merged

apm: Document sampling.tail.discard_on_write_failure config #1453

merged 8 commits into from
Jul 28, 2025

Conversation

isaacaflores2
Copy link
Contributor

@isaacaflores2 isaacaflores2 commented May 22, 2025 •
edited
Loading

⚠️ HOLD FOR 9.1 ⚠️
Document sampling.tail.discard_on_write_failure config.

I sourced the config explanation from here please let me know if the description is incorrect or unclear in any way.

Updated pages can be found in the docs preview here:

Checklist

Related issues

Part of elastic/apm-server#15330

@isaacaflores2 isaacaflores2 requested review from a team as code owners May 22, 2025 00:08
isaacaflores2
isaacaflores2 commented May 22, 2025
View reviewed changes
reference/apm/cloud/apm-settings.md Outdated
Some settings that could break your cluster if set incorrectly are blocklisted. The following settings are generally safe in cloud environments. For detailed information about APM settings, check the [APM documentation](/solutions/observability/apm/configure-apm-server.md).
::::

### Version 9.1+ [ec_version_9_1]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This config also applies to 8.19+ but I left it out based on @carsonip comment in another PR. Let me know if I should add 8.19+.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@florent-leborgne @colleenmcginnis My initial plan was to backport this PR to 8.X branch for the 8.19 release (and change the versions from 9.1 to 8.19). But I just realized 8.19 is being released before 9.1.

Should I create a separate PR for 8.X? Or do you have any other suggestions? Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @isaacaflores2. Thanks for this PR. You would need a different PR anyways for 8.19 docs because:

  • the content is likely in a different repository (https://github.com/elastic/observability-docs)
  • 8.x docs, including 8.19, are still powered by the asciidoc-based system, while 9.0 docs and above like this PR are markdown-based.

I am happy to help if you need

Copy link
Contributor Author

@isaacaflores2 isaacaflores2 Jun 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Got it thanks for sharing. I will start a PR for 8.19 docs in the other repo. I'll reach out on slack for any help

carsonip
carsonip approved these changes May 22, 2025
View reviewed changes
Copy link
Member

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

lgtm, a nit on config description. Please hold off from merging until 9.1 release

@carsonip carsonip requested a review from colleenmcginnis May 22, 2025 09:38
isaacaflores2
isaacaflores2 commented May 22, 2025
View reviewed changes
This was referenced May 23, 2025
Closed
Closed
florent-leborgne
florent-leborgne reviewed May 26, 2025
View reviewed changes
Copy link
Contributor

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

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

Thanks for the addition! I left some minor-ish styling suggestions to align the wording with our writing guidelines.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 23, 2025 •
edited
Loading

🔍 Preview links for changed docs:

🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes.

@isaacaflores2 isaacaflores2 mentioned this pull request Jun 27, 2025
Merged
10 tasks
florent-leborgne
florent-leborgne approved these changes Jun 30, 2025
View reviewed changes
Copy link
Contributor

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

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

LGTM.
Just a note that we might refactor this type of content into metadata-enriched tabs in the future to lay out this type of "per version" differences.

carsonip
carsonip reviewed Jul 4, 2025
View reviewed changes
carsonip
carsonip reviewed Jul 7, 2025
View reviewed changes
@isaacaflores2
Copy link
Contributor Author

Hmm @florent-leborgne the docs build is failing with multiple issues. I see the below message appearing a lot

Error: 'elastic/opentelemetry' links to unknown file:

Is this a known issue?

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 15, 2025 •
edited
Loading

🔍 Preview links for changed docs

@florent-leborgne
Copy link
Contributor

@isaacaflores2 It should be good now, that was unrelated to your PR and fixed some time ago

@isaacaflores2 isaacaflores2 merged commit d8a9aff into elastic:main Jul 28, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carsonip carsonip carsonip approved these changes

@florent-leborgne florent-leborgne florent-leborgne approved these changes

@colleenmcginnis colleenmcginnis Awaiting requested review from colleenmcginnis

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@isaacaflores2 @florent-leborgne @carsonip