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

ContextDataInjector ignores custom ContextDataProvider #2331

Closed
lukasalexanderweber opened this issue Mar 1, 2024 · 7 comments · Fixed by #2846
Closed

ContextDataInjector ignores custom ContextDataProvider #2331

lukasalexanderweber opened this issue Mar 1, 2024 · 7 comments · Fixed by #2846
Assignees
Labels
bug Incorrect, unexpected, or unintended behavior of existing code
Milestone

Comments

@lukasalexanderweber
Copy link

lukasalexanderweber commented Mar 1, 2024

Description

Per documentation I should be able to create a custom ContextDataProvider. However, when following the documented steps, the context is not resolved. This is also reported here.

I created a minimal reproducible example where I followed the steps described in the docs:

  1. Custom implementations should implement the org.apache.logging.log4j.core.util.ContextDataProvider interface (MyContextDataProvider)
  2. and declare it as a service by defining the implementation class in a file named META-INF/services/org.apache.logging.log4j.core.util.ContextDataProvider (org.apache.logging.log4j.core.util.ContextDataProvider)

I created a unit test with a log4j2-test.properties setup and expect for the path src/test/logs/${ctx:tenant} that the tenant is resolved by MyContextDataProvider. However, an error is printed that the file with the unresolved tenant cannot be created. It works as expected when the tenant is set via ThreadContext.put("tenant", "tenant1");

Configuration

Version: 2.23.0

Operating system: Windows 10

JDK: Corretto-17.0.8.8.1

Logs

2024-03-01T08:40:19.499742800Z main ERROR Unable to create file src/test/logs/${ctx:tenant}/logs.log java.io.IOException: Die Syntax für den Dateinamen, Verzeichnisnamen oder die Datenträgerbezeichnung ist falsch
	at java.base/java.io.WinNTFileSystem.canonicalize0(Native Method)
	at java.base/java.io.WinNTFileSystem.canonicalize(WinNTFileSystem.java:462)
	at java.base/java.io.File.getCanonicalPath(File.java:626)
	at java.base/java.io.File.getCanonicalFile(File.java:651)
	at org.apache.logging.log4j.core.util.FileUtils.makeParentDirs(FileUtils.java:141)
	at org.apache.logging.log4j.core.appender.rolling.RollingFileManager$RollingFileManagerFactory.createManager(RollingFileManager.java:863)

Reproduction

see ContextDataProviderMRE

@ppkarwasz ppkarwasz self-assigned this Mar 1, 2024
@ppkarwasz ppkarwasz added the bug Incorrect, unexpected, or unintended behavior of existing code label Mar 1, 2024
@ppkarwasz ppkarwasz changed the title Custom ContextDataProvider wont provide context ContextDataInjector ignores custom ContextDataProvider Mar 1, 2024
@ppkarwasz
Copy link
Contributor

ppkarwasz commented Mar 1, 2024

Thank you for the report.

Technically ContextDataInjector#injectContextData works correctly, so you'll see your "context" in your logs, but the ContextDataInjector#rawContextData method doesn't, so you don't see it in the context lookup. Since Windows does not accept : in path names, you see only half of the picture.

We'll look into that, although at first sight it might not be an easy improvement considering the performance restrictions.

In the meantime, you could write a StrLookup with the same data.

@rgoers
Copy link
Member

rgoers commented Mar 30, 2024

I am going to go one further. This is NOT a bug. The purpose of ContextDataProvider is to inject data into LogEvents. That is documented clearly in the Extending Log4j section of the manual. Now because the Context Lookup inspects data in the LogEvent when it is passed on it will work as you expect in that circumstance. But where no LogEvent exists it cannot possibly know where your extra data is located.

Therefore I am closing this.

@rgoers rgoers closed this as completed Mar 30, 2024
@rgoers
Copy link
Member

rgoers commented Apr 3, 2024

Well, I am going to reopen this as PR #2438 actually fixes this.

@rgoers rgoers reopened this Apr 3, 2024
@lukasalexanderweber
Copy link
Author

great! Till now we used ContextDataInjector to create the files and map the logs but we saw that it's recommended to switch to ContextDataProvider and then we were suprised that this functionality no longer works.

NOTE: It is no longer recommended that custom implementations of this interface be provided as it is difficult to do. Instead, provide a custom ContextDataProvider.

@lukasalexanderweber
Copy link
Author

@rgoers the PR you mentioned is merged, is this issue resolved?

@rgoers
Copy link
Member

rgoers commented May 7, 2024

We have not released 2.24.0 yet. You can try with the latest SNAPSHOT

@ppkarwasz
Copy link
Contributor

We postponed the merge of #2438 until 2.25.0.
However I have backported the part that fixes this issue in #2846. It is planned to be released in 2.24.0.

@ppkarwasz ppkarwasz added this to the 2.24.0 milestone Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect, unexpected, or unintended behavior of existing code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants