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

Cleanup the CSV and LDIF layouts for logback feature #294

Open
davidcoutadeur opened this issue Jun 27, 2024 · 1 comment
Open

Cleanup the CSV and LDIF layouts for logback feature #294

davidcoutadeur opened this issue Jun 27, 2024 · 1 comment

Comments

@davidcoutadeur
Copy link
Contributor

This issue is derived from #107

The goal is to fix the following issues, discovered in logback ldif and csv logging:

  • better documentation of CSV parameters: logOperations and attrs, fix outputHeader and taskNames CSV parameters,
  • fix logOperations ldif parameter, and find the utility of onlyLdif option
@davidcoutadeur
Copy link
Contributor Author

Here are the details (copy-paste from issue #107):

Here is the status of my tests about the logback configuration. I have compiled LSC from master branch (1b800cc), with openjdk 17.
I have tested on a simple use case OpenLDAP -> OpenLDAP with openjdk 17.

CSV layout

Most of the options described in the documentation are accurate:

  • File: ok
  • Append: ok
  • logOperations: works with create and update, as stated in the documentation. Also works with delete operation. (and probable changeId too) If an operation is not present here, it is not logged. We should improve the doc to specify all working operations. Note that nothing distinguish a create / update / delete operation. For example, here are the logs for a creation, update of sn, and delete operations:
uid=test,ou=people2,dc=my-domain,dc=com;test;test;;test1;;inetOrgPerson
uid=test,ou=people2,dc=my-domain,dc=com;;test;;;;
uid=test2,ou=people2,dc=my-domain,dc=com;test2;;;;;
  • attrs: ok, special attribute dn is working. when an attribute value is not present, it is empty in the line. Limitation: only the first value is displayed (see unique value inetOrgPerson above whereas 4 objectClass values were present)
  • separator: ok
  • outputHeader: does not work. A simple fix would be:
diff --git a/src/main/java/org/lsc/utils/output/CsvLayout.java b/src/main/java/org/lsc/utils/output/CsvLayout.java
index fcfc8952..226b1b25 100644
--- a/src/main/java/org/lsc/utils/output/CsvLayout.java
+++ b/src/main/java/org/lsc/utils/output/CsvLayout.java
@@ -123,6 +123,7 @@ public class CsvLayout extends LayoutBase<ILoggingEvent> {
                                                        ( taskNamesList.size() == 0 ||
                                                          taskNamesList.contains(lm.getTaskName().toLowerCase()))) {
                                StringBuilder sb = new StringBuilder(1024);
+                               sb.append(this.getHeader());
 
                                Map<String, List<Object>> modifications = lm.getModificationsItemsByHash();
 
diff --git a/src/test/java/org/lsc/utils/output/CsvLayoutTest.java b/src/test/java/org/lsc/utils/output/CsvLayoutTest.java
index 65c27bbd..a9f81c35 100644
--- a/src/test/java/org/lsc/utils/output/CsvLayoutTest.java
+++ b/src/test/java/org/lsc/utils/output/CsvLayoutTest.java
@@ -178,10 +178,10 @@ public class CsvLayoutTest {
                // log one line to check that the outputHeader is prepended
                ILoggingEvent event = makeLoggingEvent(jm.toString(), jm);
                assertEquals("givenName%sn%dn%%cn\n", layout.getHeader());
-               assertEquals("Jon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
+               assertEquals("givenName%sn%dn%%cn\nJon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
 
                // log the same line again to check that the outputHeader is not logged again
                event = makeLoggingEvent(jm.toString(), jm);
-               assertEquals("Jon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
+               assertEquals("givenName%sn%dn%%cn\nJon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
        }
 }

Note: there is actually a test /src/test/java/org/lsc/utils/output/CsvLayoutTest.java, but it just verifies the method getHeader() is returning the corresponding header. But the getHeader() method does not seem to be called anywhere in our code. If we apply the fix above, we must also fix this test.

  • taskNames: does not seem to work. Setting a dummy task name here still produces a CSV output. The taskNames parameter is empty in CsvLayout.java.

LDIF layout

Globally, the ldif output is working.

  • File: ok
  • Append: ok
  • logOperations: whatever operation is set up, all operations are logged. (create, update, delete) We need to fix this.
  • onlyLdif: no idea what this option does. The output seems identical for true or false values.

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

No branches or pull requests

1 participant