Skip to content

Add collectionName and databaseName attributes to MongoDbProvider #3702

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

Merged
merged 3 commits into from
May 31, 2025

Conversation

vy
Copy link
Member

@vy vy commented May 31, 2025

This work is a clone of #3322 authored by @jesmith17. Commit signatures were missing, Spotless was failing, and author could not address these issues. I created a separate PR to avoid truncating the git & GitHub conversation history in the old PR.

Copy link
Contributor

@ppkarwasz ppkarwasz 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, thanks for fixing #3322!

Just a quick note on the tests in the log4j-mongodb module:

  • The newly added integration tests (CollectionNameIT, DatabaseAndCollectionNameIT, and NoDatabaseAndCollectionNameIT) seem somewhat redundant. From what I can tell, the existing ProviderTest unit test already verifies that the correct strings are passed to the MongoDB driver, so these ITs might not add much value beyond that.
  • Not directly related to this PR, but something we might want to track: the CappedIntIT and CappedLongIT tests don’t appear to actually verify that the collection created is capped. It could be worth opening an issue to address that.

When I get a chance, I’ll try porting this PR over to the 2.x branch. After that, maybe we can switch roles—I'll open the PR, and you can do the honors of reviewing. 😉

@vy
Copy link
Member Author

vy commented May 31, 2025

@ppkarwasz, removed CollectionNameIT and DatabaseAndCollectionNameIT. (I couldn't understand what you meant with NoDatabaseAndCollectionNameIT.)

@vy vy enabled auto-merge (squash) May 31, 2025 18:35
@vy vy merged commit 2b08de6 into main May 31, 2025
7 checks passed
@vy vy deleted the jesmith17-main-mongo-dbname branch May 31, 2025 18:47
@github-project-automation github-project-automation bot moved this from To triage to Done in Log4j bug tracker May 31, 2025
@ppkarwasz ppkarwasz modified the milestones: 2.25.0, 3.0.0-beta4 May 31, 2025
@ppkarwasz
Copy link
Contributor

I couldn't understand what you meant with NoDatabaseAndCollectionNameIT.

I think I was mistakenly looking at the 2.x branch: https://github.com/apache/logging-log4j2/tree/2.x/log4j-mongodb/src/test/java/org/apache/logging/log4j/mongodb

BTW: regarding 2.x you already introduced this change several months ago: #3467. I don't know why I did add this PR to milestone 2.25.0.

@vy
Copy link
Member Author

vy commented May 31, 2025

regarding 2.x you already introduced this change several months ago: #3467. I don't know why I did add this PR to milestone 2.25.0.

Doh! Never occurred to me either! 🤦‍♂️ I guess we are both experiencing "burnout as a life style"? 😅

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

Successfully merging this pull request may close these issues.

3 participants