Skip to content

Replayer logging changes #1463

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

mikaylathompson
Copy link
Collaborator

@mikaylathompson mikaylathompson commented Apr 18, 2025

Description

Behavior is as follows:

  • all categories of logs are written to a local directory until they're ready to be rolled over
  • then they're compressed and written to the shared logs volume

This PR initially included code that looked for an env var to decide which log4j2 properties file to use. It turns out that exists natively in Log4j, so we're using that instead.

Issues Resolved

Testing

Testing needed! Discussed with @gregschohn to add tests that validate the new logging properties file path behavior, especially when the file doesn't exist.

Check List

  • New functionality includes testing
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@AndreKurait
Copy link
Member

@mikaylathompson, i've made some updates for the default config here: mikaylathompson#6 Let me know what you think

Signed-off-by: Mikayla Thompson <[email protected]>
This reverts commit 54d5c46.

Signed-off-by: Mikayla Thompson <[email protected]>
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Two considerations, but if @AndreKurait and @mikaylathompson are happy with this I'm fine to merge.

1/ Should these changes be replicated in the RFS logger?
2/ The complexity of this file is growing, almost seems like having a script/programmatic construction would be useful for reuse

@mikaylathompson mikaylathompson merged commit 8155c04 into opensearch-project:main May 9, 2025
57 checks passed
@mikaylathompson mikaylathompson deleted the replayer-logging-changes branch May 9, 2025 15:32
Copy link

codecov bot commented May 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (ee40cc6) to head (8b563f9).
Report is 37 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff      @@
##   main   #1463   +/-   ##
============================
============================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants